Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-11 Thread Zach Brown
> Yeah, but that means the completion has to be delivered from process > context. That's not what aio does today, and it'd be a real performance > regression. It'd only have to to complete from process context if it faults. The cheapest possible delivery mechanism is simple cpu stores. In the

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-11 Thread Zach Brown
Yeah, but that means the completion has to be delivered from process context. That's not what aio does today, and it'd be a real performance regression. It'd only have to to complete from process context if it faults. The cheapest possible delivery mechanism is simple cpu stores. In the vast

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-10 Thread Kent Overstreet
On Wed, Oct 10, 2012 at 02:43:15PM -0700, Zach Brown wrote: > > True. But that could be solved with a separate interface that either > > doesn't use a context to submit a call synchronously, or uses an > > implicit per thread context. > > Sure, but why bother if we can make the one submission

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-10 Thread Zach Brown
> True. But that could be solved with a separate interface that either > doesn't use a context to submit a call synchronously, or uses an > implicit per thread context. Sure, but why bother if we can make the one submission interface fast enough to satisfy quick callers? Less is more, and all

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-10 Thread Zach Brown
True. But that could be solved with a separate interface that either doesn't use a context to submit a call synchronously, or uses an implicit per thread context. Sure, but why bother if we can make the one submission interface fast enough to satisfy quick callers? Less is more, and all that.

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-10 Thread Kent Overstreet
On Wed, Oct 10, 2012 at 02:43:15PM -0700, Zach Brown wrote: True. But that could be solved with a separate interface that either doesn't use a context to submit a call synchronously, or uses an implicit per thread context. Sure, but why bother if we can make the one submission interface

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 05:26:34PM -0700, Zach Brown wrote: > > The AIO ringbuffer stuff just annoys me more than most > > Not more than everyone, though, I can personally promise you that :). > > > (it wasn't until > > the other day that I realized it was actually exported to userspace... > >

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
> The AIO ringbuffer stuff just annoys me more than most Not more than everyone, though, I can personally promise you that :). > (it wasn't until > the other day that I realized it was actually exported to userspace... > what led to figuring that out was noticing aio_context_t was a ulong, > and

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 04:10:59PM -0700, Zach Brown wrote: > > Well, the ringbuffer does have those compat flags and incompat flags. > > Which libaio conveniently doesn't check, but for what it does it > > shouldn't really matter I guess. > > Well, the presumed point of the incompat flags would

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
> Well, the ringbuffer does have those compat flags and incompat flags. > Which libaio conveniently doesn't check, but for what it does it > shouldn't really matter I guess. Well, the presumed point of the incompat flags would be to tell an app that it isn't going to get what it expects! Ideally

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 03:47:03PM -0700, Zach Brown wrote: > > If libaio is the only thing in userspace looking at the ringbuffer, and > > if I'm looking at the latest libaio code this shouldn't break > > anything... > > We can't assume that libaio is the only thing in userspace using the >

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
> If libaio is the only thing in userspace looking at the ringbuffer, and > if I'm looking at the latest libaio code this shouldn't break > anything... We can't assume that libaio is the only thing in userspace using the mapped buffer -- as scary a thought as that is :). If we wanted to change

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 11:37:53AM -0700, Zach Brown wrote: > On Mon, Oct 08, 2012 at 11:39:20PM -0700, Kent Overstreet wrote: > > Bunch of cleanup > > Ugh. That's way too much noisy change for one patch with no > description. Break it up into functional pieces and actually describe > them.

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
On Mon, Oct 08, 2012 at 11:39:20PM -0700, Kent Overstreet wrote: > Bunch of cleanup Ugh. That's way too much noisy change for one patch with no description. Break it up into functional pieces and actually describe them. > events off the ringbuffer without racing with io_getevents(). Are you

[PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
Bunch of cleanup, and make it lockless so that userspace can safely pull events off the ringbuffer without racing with io_getevents(). Signed-off-by: Kent Overstreet --- fs/aio.c | 220 +- 1 file changed, 73 insertions(+), 147

[PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
Bunch of cleanup, and make it lockless so that userspace can safely pull events off the ringbuffer without racing with io_getevents(). Signed-off-by: Kent Overstreet koverstr...@google.com --- fs/aio.c | 220 +- 1 file changed, 73

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
On Mon, Oct 08, 2012 at 11:39:20PM -0700, Kent Overstreet wrote: Bunch of cleanup Ugh. That's way too much noisy change for one patch with no description. Break it up into functional pieces and actually describe them. events off the ringbuffer without racing with io_getevents(). Are you

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 11:37:53AM -0700, Zach Brown wrote: On Mon, Oct 08, 2012 at 11:39:20PM -0700, Kent Overstreet wrote: Bunch of cleanup Ugh. That's way too much noisy change for one patch with no description. Break it up into functional pieces and actually describe them. Heh, I

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
If libaio is the only thing in userspace looking at the ringbuffer, and if I'm looking at the latest libaio code this shouldn't break anything... We can't assume that libaio is the only thing in userspace using the mapped buffer -- as scary a thought as that is :). If we wanted to change the

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 03:47:03PM -0700, Zach Brown wrote: If libaio is the only thing in userspace looking at the ringbuffer, and if I'm looking at the latest libaio code this shouldn't break anything... We can't assume that libaio is the only thing in userspace using the mapped buffer

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
Well, the ringbuffer does have those compat flags and incompat flags. Which libaio conveniently doesn't check, but for what it does it shouldn't really matter I guess. Well, the presumed point of the incompat flags would be to tell an app that it isn't going to get what it expects! Ideally

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 04:10:59PM -0700, Zach Brown wrote: Well, the ringbuffer does have those compat flags and incompat flags. Which libaio conveniently doesn't check, but for what it does it shouldn't really matter I guess. Well, the presumed point of the incompat flags would be to

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Zach Brown
The AIO ringbuffer stuff just annoys me more than most Not more than everyone, though, I can personally promise you that :). (it wasn't until the other day that I realized it was actually exported to userspace... what led to figuring that out was noticing aio_context_t was a ulong, and got

Re: [PATCH 5/5] aio: Refactor aio_read_evt, use cmxchg(), fix bug

2012-10-09 Thread Kent Overstreet
On Tue, Oct 09, 2012 at 05:26:34PM -0700, Zach Brown wrote: The AIO ringbuffer stuff just annoys me more than most Not more than everyone, though, I can personally promise you that :). (it wasn't until the other day that I realized it was actually exported to userspace... what led to