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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
