Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664588 > unrolled thread

Re: [PATCH] aio: Add command to wait completion of all requests

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] aio: Add command to wait completion of all requests Cyrill Gorcunov <gorcunov@gmail.com> - 2017-06-13 10:20 +0200
    Re: [PATCH] aio: Add command to wait completion of all requests Cyrill Gorcunov <gorcunov@gmail.com> - 2017-06-13 12:40 +0200

#1664588 — Re: [PATCH] aio: Add command to wait completion of all requests

On Fri, Jun 09, 2017 at 12:49:34PM +0300, Kirill Tkhai wrote:
...
> +static int aio_wait_all(struct kioctx *ctx)
> +{
> +	unsigned users, reqs = 0;
> +	struct kioctx_cpu *kcpu;
> +	int cpu, ret;
> +
> +	if (atomic_xchg(&ctx->dead, 1))
> +		return -EBUSY;
> +
> +	users = atomic_read(&current->mm->mm_users);
> +	if (users > 1) {
> +		/*
> +		 * Wait till concurrent threads and aio_complete() see
> +		 * dead flag. Implies full memory barrier on all cpus.
> +		 */
> +		synchronize_sched();
> +	} else {
> +		/*
> +		 * Sync with aio_complete() to be sure it puts reqs_available,
> +		 * when dead flag is already seen.
> +		 */
> +		spin_lock_irq(&ctx->completion_lock);
> +	}
> +
> +	for_each_possible_cpu(cpu) {
> +		kcpu = per_cpu_ptr(ctx->cpu, cpu);
> +		reqs += kcpu->reqs_available;

I'm not that familiar with AIO internals but this snippet worries me:
the reqs_available is unsigned int, reqs is unsigned it as well but
used as an accumulator over ALL cpus, can't it get overflow and
gives modulo result, should not it be unsigned long or something?

> +		kcpu->reqs_available = 0;
> +	}
> +
> +	if (users == 1)
> +		spin_unlock_irq(&ctx->completion_lock);
> +
> +	atomic_add(reqs, &ctx->reqs_available);
> +
> +	ret = wait_event_interruptible(ctx->wait, reqs_completed(ctx));
> +
> +	atomic_set(&ctx->dead, 0);
> +
> +	return ret;
> +}

[toc] | [next] | [standalone]

#1664727

On Tue, Jun 13, 2017 at 12:45:39PM +0300, Kirill Tkhai wrote:
> > 
> > I'm not that familiar with AIO internals but this snippet worries me:
> > the reqs_available is unsigned int, reqs is unsigned it as well but
> > used as an accumulator over ALL cpus, can't it get overflow and
> > gives modulo result, should not it be unsigned long or something?
> 
> All available reqs are initially contain in kioctx::reqs_available,
> which is atomic_t, and then they are distributed over percpu counters.
> So, this is OK.

Thanks for explanation! Looks ok to me. But I'm far from being
aio expert, if someone else won't beat me I'll take a closer
look in [a day|couple of] or so.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web