Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-30 Thread Linus Torvalds
On Tue, 30 Oct 2007, Zach Brown wrote: > > OK, I tested and verified Karl's fix and wrote some commentary around it. Thanks, will apply. > (Would a aio-dio git repo on kernel.org for these kind of fixes be well > received?) If it's one of these "in a blue moon" things I don't think it matters.

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-30 Thread Zach Brown
> Yes, I do - I'd been tripping over it once every couple weeks, > and I finally figured out how to hold my mouth right so that it > fails (almost) every time. OK, I tested and verified Karl's fix and wrote some commentary around it. (Would a aio-dio git repo on kernel.org for these kind of fixes

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Zach Brown
Linus Torvalds wrote: > > On Fri, 26 Oct 2007, Zach Brown wrote: >> I can throw together a patch if you haven't already committed one by the >> time you read this ;). > > I'm not touching that code except to send out possible patches for others > to test and comment on. I have no test-cases, nor

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Karl Schendel
Linus Torvalds wrote: > > On Fri, 26 Oct 2007, Zach Brown wrote: >> I can throw together a patch if you haven't already committed one by the >> time you read this ;). > > I'm not touching that code except to send out possible patches for others > to test and comment on. I have no test-cases, nor

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Linus Torvalds
On Fri, 26 Oct 2007, Zach Brown wrote: > > I can throw together a patch if you haven't already committed one by the > time you read this ;). I'm not touching that code except to send out possible patches for others to test and comment on. I have no test-cases, nor any real interest in it. So

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Zach Brown
Linus Torvalds wrote: > > On Fri, 26 Oct 2007, Zach Brown wrote: >> I think that test should be changed to > > How about not testing at all? Which was what the old code did. > > Just do the invalidate unconditionally for any writes, and screw the end > result of the invalidate, since we cannot

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Linus Torvalds
On Fri, 26 Oct 2007, Zach Brown wrote: > > I think that test should be changed to How about not testing at all? Which was what the old code did. Just do the invalidate unconditionally for any writes, and screw the end result of the invalidate, since we cannot afford to overwrite the previous

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Karl Schendel
Zach Brown wrote: > Linus Torvalds wrote: >> Hmm. If I read this right, this bug seems to have been introduced by >> commit 65b8291c4000e5f38fc94fb2ca0cb7e8683c8a1b ("dio: invalidate clean >> pages before dio write") back in March. > > Agreed. And it's a really dumb bug. ->direct_io will almos

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Zach Brown
Linus Torvalds wrote: > Hmm. If I read this right, this bug seems to have been introduced by > commit 65b8291c4000e5f38fc94fb2ca0cb7e8683c8a1b ("dio: invalidate clean > pages before dio write") back in March. Agreed. And it's a really dumb bug. ->direct_io will almost always return -EIOCBQUEUE

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Karl Schendel
Linus Torvalds wrote: > So we may > actually end up doing some IO, but then returning the "wrong" error code > from the invalidate. Hmm? > A point. In an all-seeing, all-caring universe, it would be the read hitting the cached page that couldn't be invalidated that would get the error, not

Re: [PATCH] Fix bad data from non-direct-io read after direct-io write

2007-10-26 Thread Linus Torvalds
Hmm. If I read this right, this bug seems to have been introduced by commit 65b8291c4000e5f38fc94fb2ca0cb7e8683c8a1b ("dio: invalidate clean pages before dio write") back in March. Before that, we'd call invalidate_inode_pages2_range() unconditionally after the call mapping->a_ops->direct_IO()