On 4/11/07, Zach Brown <[EMAIL PROTECTED]> wrote:
First, I'll NAK this and all AIO patches until the patch description says that it's been run through the regression tests that we've started collecting in autotest. They're trivial to run, never fear:
OK. I will run those regression tests.
> Resurrect an old patch that uses atomic operation to update ring buffer > index on AIO event queue. This work allows futher application/libaio > optimization to run fast path io_getevents in user space. I have mixed feelings. I think the userspace getevents support was poorly designed and the simple fact that we've gone this long without it says just how desperately the feature isn't needed.
I kept on getting requests from application developers who want that feature. My initial patch was dated back May 2004.
We're allocating more ring elements than userspace asked for and than were accounted for in aio_max_nr. That cost, however teeny, will be measured against the benefit.
I noticed that while writing the patch. We have the same bug right now that nr_events is enlarged to consume the entire mmap'ed ring buffer pages. But yet, we don't account those in aio_max_nr. I didn't fix that up in this patch because I was going to do a separate patch on that.
> - /* Compensate for the ring buffer's head/tail overlap entry */ > - nr_events += 2; /* 1 is required, 2 for good luck */ > + /* round nr_event to next power of 2 */ > + nr_events = roundup_pow_of_two(nr_events); Is that buggy? How will the code tell if head == tail means a full ring or an empty ring? The old code added that extra event to let it tell the ring was full before head == tail and it would think it's empty again, I think. I'm guessing roundup(nr_events + 1) is needed.
I don't think it's a bug. akpm taught me the trick: we don't wrap the index around the ring size in this scheme, so the extra space is not needed.
The ring page, which is mmaped to userspace at some weird address, was just written through a kmap. Do we need a flush_dcache_page()? This isn't this patch's problem, but it'd need to be addressed if we're using the ring from userspace.
I will look into this aside from this patch. - Ken - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
