The patch titled Subject: aio: fix ringbuffer calculation so we don't wrap has been added to the -mm tree. Its filename is aio-fix-ringbuffer-calculation-so-we-dont-wrap.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/SubmitChecklist when testing your code *** The -mm tree is included into linux-next and is updated there every 3-4 working days ------------------------------------------------------ From: Kent Overstreet <koverstreet@xxxxxxxxxx> Subject: aio: fix ringbuffer calculation so we don't wrap When calculating the size of the ringbuffer, aio_setup_ring() adds 2 to nr_events so that the head and tail entries don't overlap when the ringbuffer fills up. But then later the code uses the calculated ringbuffer size for the maximum number of outstanding kiocbs, so this buffer doesn't actually get used. Ouch. Fix it so the ringbuffer size and maximum number of requests are distinct again, and document things while we're at it. Signed-off-by: Kent Overstreet <koverstreet@xxxxxxxxxx> Cc: Benjamin LaHaise <bcrl@xxxxxxxxx> Cc: Zach Brown <zab@xxxxxxxxxx> Cc: Josh Boyer <jwboyer@xxxxxxxxxx> Cc: Zach Brown <zab@xxxxxxxxxx> Cc: Theodore Ts'o <tytso@xxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- fs/aio.c | 69 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 23 deletions(-) diff -puN fs/aio.c~aio-fix-ringbuffer-calculation-so-we-dont-wrap fs/aio.c --- a/fs/aio.c~aio-fix-ringbuffer-calculation-so-we-dont-wrap +++ a/fs/aio.c @@ -75,13 +75,21 @@ struct kioctx { struct __percpu kioctx_cpu *cpu; - unsigned req_batch; - - unsigned nr; + /* Size of ringbuffer, in units of struct io_event */ + unsigned nr_events; - /* sys_io_setup currently limits this to an unsigned int */ + /* + * Maximum number of outstanding requests: + * sys_io_setup currently limits this to an unsigned int + */ unsigned max_reqs; + /* + * For percpu reqs_available, number of slots we move to/from global + * counter at a time: + */ + unsigned req_batch; + unsigned long mmap_base; unsigned long mmap_size; @@ -92,6 +100,14 @@ struct kioctx { struct work_struct rcu_work; struct { + /* + * This counts the number of available slots in the ringbuffer, + * so we avoid overflowing it: it's decremented (if positive) + * when allocating a kiocb and incremented when the resulting + * io_event is pulled off the ringbuffer. + * + * We batch accesses to it with a percpu version. + */ atomic_t reqs_available; } ____cacheline_aligned_in_smp; @@ -172,9 +188,6 @@ static int aio_setup_ring(struct kioctx unsigned long size, populate; int nr_pages; - nr_events = max(nr_events, num_possible_cpus() * 4); - nr_events *= 2; - /* Compensate for the ring buffer's head/tail overlap entry */ nr_events += 2; /* 1 is required, 2 for good luck */ @@ -187,7 +200,7 @@ static int aio_setup_ring(struct kioctx nr_events = (PAGE_SIZE * nr_pages - sizeof(struct aio_ring)) / sizeof(struct io_event); - ctx->nr = 0; + ctx->nr_events = 0; ctx->ring_pages = ctx->internal_pages; if (nr_pages > AIO_RING_PAGES) { ctx->ring_pages = kcalloc(nr_pages, sizeof(struct page *), @@ -222,7 +235,7 @@ static int aio_setup_ring(struct kioctx mm_populate(ctx->mmap_base, populate); ctx->user_id = ctx->mmap_base; - ctx->nr = nr_events; /* trusted copy */ + ctx->nr_events = nr_events; /* trusted copy */ ring = kmap_atomic(ctx->ring_pages[0]); ring->nr = nr_events; /* user copy */ @@ -334,20 +347,20 @@ static void free_ioctx(struct kioctx *ct head = ring->head; kunmap_atomic(ring); - while (atomic_read(&ctx->reqs_available) < ctx->nr) { + while (atomic_read(&ctx->reqs_available) < ctx->max_reqs) { wait_event(ctx->wait, (head != ctx->shadow_tail) || - (atomic_read(&ctx->reqs_available) >= ctx->nr)); + (atomic_read(&ctx->reqs_available) >= ctx->max_reqs)); avail = (head <= ctx->shadow_tail ? - ctx->shadow_tail : ctx->nr) - head; + ctx->shadow_tail : ctx->nr_events) - head; atomic_add(avail, &ctx->reqs_available); head += avail; - head %= ctx->nr; + head %= ctx->nr_events; } - WARN_ON(atomic_read(&ctx->reqs_available) > ctx->nr); + WARN_ON(atomic_read(&ctx->reqs_available) > ctx->max_reqs); aio_free_ring(ctx); @@ -383,6 +396,18 @@ static struct kioctx *ioctx_alloc(unsign struct kioctx *ctx; int err = -ENOMEM; + /* + * We keep track of the number of available ringbuffer slots, to prevent + * overflow (reqs_available), and we also use percpu counters for this. + * + * So since up to half the slots might be on other cpu's percpu counters + * and unavailable, double nr_events so userspace sees what they + * expected: additionally, we move req_batch slots to/from percpu + * counters at a time, so make sure that isn't 0: + */ + nr_events = max(nr_events, num_possible_cpus() * 4); + nr_events *= 2; + /* Prevent overflows */ if ((nr_events > (0x10000000U / sizeof(struct io_event))) || (nr_events > (0x10000000U / sizeof(struct kiocb)))) { @@ -398,6 +423,8 @@ static struct kioctx *ioctx_alloc(unsign return ERR_PTR(-ENOMEM); ctx->max_reqs = nr_events; + atomic_set(&ctx->reqs_available, nr_events); + ctx->req_batch = nr_events / (num_possible_cpus() * 4); percpu_ref_init(&ctx->users); rcu_read_lock(); @@ -417,10 +444,6 @@ static struct kioctx *ioctx_alloc(unsign if (aio_setup_ring(ctx) < 0) goto out_freepcpu; - atomic_set(&ctx->reqs_available, ctx->nr); - ctx->req_batch = ctx->nr / (num_possible_cpus() * 4); - BUG_ON(!ctx->req_batch); - /* limit the number of system wide aios */ spin_lock(&aio_nr_lock); if (aio_nr + nr_events > aio_max_nr || @@ -437,7 +460,7 @@ static struct kioctx *ioctx_alloc(unsign spin_unlock(&mm->ioctx_lock); pr_debug("allocated ioctx %p[%ld]: mm=%p mask=0x%x\n", - ctx, ctx->user_id, mm, ctx->nr); + ctx, ctx->user_id, mm, ctx->nr_events); return ctx; out_cleanup: @@ -657,7 +680,7 @@ static inline unsigned kioctx_ring_put(s struct io_event *ev_page, *event; unsigned pos = tail + AIO_EVENTS_OFFSET; - if (++tail >= ctx->nr) + if (++tail >= ctx->nr_events) tail = 0; ev_page = kmap_atomic(ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE]); @@ -871,14 +894,14 @@ static long aio_read_events_ring(struct head = ring->head; kunmap_atomic(ring); - pr_debug("h%u t%u m%u\n", head, ctx->shadow_tail, ctx->nr); + pr_debug("h%u t%u m%u\n", head, ctx->shadow_tail, ctx->nr_events); if (head == ctx->shadow_tail) goto out; while (ret < nr) { long avail = (head <= ctx->shadow_tail - ? ctx->shadow_tail : ctx->nr) - head; + ? ctx->shadow_tail : ctx->nr_events) - head; struct io_event *ev; struct page *page; @@ -904,7 +927,7 @@ static long aio_read_events_ring(struct ret += avail; head += avail; - head %= ctx->nr; + head %= ctx->nr_events; } ring = kmap_atomic(ctx->ring_pages[0]); _ Patches currently in -mm which might be from koverstreet@xxxxxxxxxx are mm-remove-old-aio-use_mm-comment.patch aio-remove-dead-code-from-aioh.patch gadget-remove-only-user-of-aio-retry.patch aio-remove-retry-based-aio.patch char-add-aio_readwrite-to-dev-nullzero.patch aio-kill-return-value-of-aio_complete.patch aio-kiocb_cancel.patch aio-move-private-stuff-out-of-aioh.patch aio-dprintk-pr_debug.patch aio-do-fget-after-aio_get_req.patch aio-make-aio_put_req-lockless.patch aio-refcounting-cleanup.patch wait-add-wait_event_hrtimeout.patch aio-make-aio_read_evt-more-efficient-convert-to-hrtimers.patch aio-use-flush_dcache_page.patch aio-use-cancellation-list-lazily.patch aio-change-reqs_active-to-include-unreaped-completions.patch aio-kill-batch-allocation.patch aio-kill-struct-aio_ring_info.patch aio-give-shared-kioctx-fields-their-own-cachelines.patch aio-reqs_active-reqs_available.patch aio-percpu-reqs_available.patch generic-dynamic-per-cpu-refcounting.patch aio-percpu-ioctx-refcount.patch aio-use-xchg-instead-of-completion_lock.patch aio-dont-include-aioh-in-schedh.patch aio-kill-ki_key.patch aio-kill-ki_retry.patch block-aio-batch-completion-for-bios-kiocbs.patch virtio-blk-convert-to-batch-completion.patch mtip32xx-convert-to-batch-completion.patch aio-fix-aio_read_events_ring-types.patch aio-document-clarify-aio_read_events-and-shadow_tail.patch aio-correct-calculation-of-available-events.patch aio-v2-fix-kioctx-not-being-freed-after-cancellation-at-exit-time.patch aio-fix-ringbuffer-calculation-so-we-dont-wrap.patch -- To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html