Messages in this thread | | | Subject | Re: [PATCH] aio: Add command to wait completion of all requests | From | Kirill Tkhai <> | Date | Tue, 13 Jun 2017 12:45:39 +0300 |
| |
On 13.06.2017 11:14, Cyrill Gorcunov wrote: > 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(¤t->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?
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.
>> + 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; >> +}
| |