Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 10:40 -0800, Zach Brown wrote: > >> There are some strange O_DIRECT corner cases in here such that the > >> 'last > >> writer' may actually be a 'last reader' and winning can mean have > >> a copy > >> of the page in page cache older than the copy on disk. > > > > As long

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Zach Brown
There are some strange O_DIRECT corner cases in here such that the 'last writer' may actually be a 'last reader' and winning can mean have a copy of the page in page cache older than the copy on disk. As long as it is marked dirty so that it eventually gets synced to disk, it shouldn't

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 11:30 -0500, Trond Myklebust wrote: > > One option is to have invalidate_inode_pages2_range continue if it can't > > toss a page but still return something that O_DIRECT ignores (living > > with the race), but it looks like I can make a launder_page op that does > > the right

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 11:08 -0500, Chris Mason wrote: > On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: > > On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: > > > On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: > > > > aio is not responsible for this

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Chris Mason
On Tue, Feb 20, 2007 at 05:06:47PM +0100, Arjan van de Ven wrote: > > We don't try to resolve "conflicting" writes between ordinary mmap() and > > write(), so why should we be doing it for mmap and O_DIRECT? > > > > mmap() is designed to violate the ordinary mutex locks for write(), so > > if a

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Chris Mason
On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: > On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: > > On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: > > > aio is not responsible for this particular synchronization. Those fixes > > > (if we make them)

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Arjan van de Ven
> We don't try to resolve "conflicting" writes between ordinary mmap() and > write(), so why should we be doing it for mmap and O_DIRECT? > > mmap() is designed to violate the ordinary mutex locks for write(), so > if a conflict arises, whether it be with O_DIRECT or ordinary writes > then it is

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Benjamin LaHaise
On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: > We don't try to resolve "conflicting" writes between ordinary mmap() and > write(), so why should we be doing it for mmap and O_DIRECT? Yes we do -- both writes will succeed. More importantly, if one modifies the first 512 bytes

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: > On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: > > aio is not responsible for this particular synchronization. Those fixes > > (if we make them) should come from other places. The patch is important > > to get aio error

RE: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Ananiev, Leonid I
ilto:[EMAIL PROTECTED] Sent: Monday, February 19, 2007 11:35 PM To: [EMAIL PROTECTED]; linux-kernel@vger.kernel.org Cc: Benjamin LaHaise; Suparna bhattacharya; Andrew Morton; Ananiev, Leonid I Subject: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event aio: propogate post-EI

RE: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Ananiev, Leonid I
, 2007 11:35 PM To: [EMAIL PROTECTED]; linux-kernel@vger.kernel.org Cc: Benjamin LaHaise; Suparna bhattacharya; Andrew Morton; Ananiev, Leonid I Subject: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event aio: propogate post-EIOCBQUEUED errors to completion event This addresses

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: aio is not responsible for this particular synchronization. Those fixes (if we make them) should come from other places. The patch is important to get aio error handling

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Benjamin LaHaise
On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: We don't try to resolve conflicting writes between ordinary mmap() and write(), so why should we be doing it for mmap and O_DIRECT? Yes we do -- both writes will succeed. More importantly, if one modifies the first 512 bytes of

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Arjan van de Ven
We don't try to resolve conflicting writes between ordinary mmap() and write(), so why should we be doing it for mmap and O_DIRECT? mmap() is designed to violate the ordinary mutex locks for write(), so if a conflict arises, whether it be with O_DIRECT or ordinary writes then it is a case

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Chris Mason
On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: aio is not responsible for this particular synchronization. Those fixes (if we make them) should come

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Chris Mason
On Tue, Feb 20, 2007 at 05:06:47PM +0100, Arjan van de Ven wrote: We don't try to resolve conflicting writes between ordinary mmap() and write(), so why should we be doing it for mmap and O_DIRECT? mmap() is designed to violate the ordinary mutex locks for write(), so if a conflict

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 11:08 -0500, Chris Mason wrote: On Tue, Feb 20, 2007 at 11:01:50AM -0500, Trond Myklebust wrote: On Mon, 2007-02-19 at 19:21 -0500, Benjamin LaHaise wrote: On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: aio is not responsible for this particular

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 11:30 -0500, Trond Myklebust wrote: One option is to have invalidate_inode_pages2_range continue if it can't toss a page but still return something that O_DIRECT ignores (living with the race), but it looks like I can make a launder_page op that does the right thing.

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Zach Brown
There are some strange O_DIRECT corner cases in here such that the 'last writer' may actually be a 'last reader' and winning can mean have a copy of the page in page cache older than the copy on disk. As long as it is marked dirty so that it eventually gets synced to disk, it shouldn't

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-20 Thread Trond Myklebust
On Tue, 2007-02-20 at 10:40 -0800, Zach Brown wrote: There are some strange O_DIRECT corner cases in here such that the 'last writer' may actually be a 'last reader' and winning can mean have a copy of the page in page cache older than the copy on disk. As long as it is marked

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Chris Mason
On Mon, Feb 19, 2007 at 07:21:09PM -0500, Benjamin LaHaise wrote: > On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: > > aio is not responsible for this particular synchronization. Those fixes > > (if we make them) should come from other places. The patch is important > > to get aio

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
I would argue that one common cause of the EIO is userland error (mmap concurrent with O_DIRECT), and EIO is the correct answer. I disagree. That means that using the pagecache to synchronize things like the proposed online defragmentation will occasionally make O_DIRECT users fail.

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Benjamin LaHaise
On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: > aio is not responsible for this particular synchronization. Those fixes > (if we make them) should come from other places. The patch is important > to get aio error handling right. > > I would argue that one common cause of the EIO

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Chris Mason
On Mon, Feb 19, 2007 at 11:58:16PM +0300, Ananiev, Leonid I wrote: > > while triggering EIO in invalidate_inode_pages2_range() > ... > > With this patch aio-stress sees -EIO. > > Actually if invalidate_inode_pages2_range() returns EIO it means > that internal kernel synchronization conflict was

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
I *think* the patch is right, but picking the changes to the code and watching its movement at the same time is making my head spin. Really? The only things that changed are the assignment from iocb- >res (after testing pending_err) instead of the 'res' argument and the dprintk.

RE: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Ananiev, Leonid I
ubject: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event aio: propogate post-EIOCBQUEUED errors to completion event This addresses an oops reported by Leonid Ananiev <[EMAIL PROTECTED]> as archived at http://lkml.org/lkml/2007/2/8/337. O_DIRECT kicks off bios and returns

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Benjamin LaHaise
On Mon, Feb 19, 2007 at 12:35:27PM -0800, Zach Brown wrote: > aio: propogate post-EIOCBQUEUED errors to completion event This patch has to be split in 2 to make it easier to review -- the change moving the event insertion code for the completion queue should stand separat

[PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
aio: propogate post-EIOCBQUEUED errors to completion event This addresses an oops reported by Leonid Ananiev <[EMAIL PROTECTED]> as archived at http://lkml.org/lkml/2007/2/8/337. O_DIRECT kicks off bios and returns -EIOCBQUEUED to indicate its intention to call aio_complete() once the bios

[PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
aio: propogate post-EIOCBQUEUED errors to completion event This addresses an oops reported by Leonid Ananiev [EMAIL PROTECTED] as archived at http://lkml.org/lkml/2007/2/8/337. O_DIRECT kicks off bios and returns -EIOCBQUEUED to indicate its intention to call aio_complete() once the bios

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Benjamin LaHaise
On Mon, Feb 19, 2007 at 12:35:27PM -0800, Zach Brown wrote: aio: propogate post-EIOCBQUEUED errors to completion event This patch has to be split in 2 to make it easier to review -- the change moving the event insertion code for the completion queue should stand separately, as it is completely

RE: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Ananiev, Leonid I
: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event aio: propogate post-EIOCBQUEUED errors to completion event This addresses an oops reported by Leonid Ananiev [EMAIL PROTECTED] as archived at http://lkml.org/lkml/2007/2/8/337. O_DIRECT kicks off bios and returns -EIOCBQUEUED

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
I *think* the patch is right, but picking the changes to the code and watching its movement at the same time is making my head spin. Really? The only things that changed are the assignment from iocb- res (after testing pending_err) instead of the 'res' argument and the dprintk.

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Chris Mason
On Mon, Feb 19, 2007 at 11:58:16PM +0300, Ananiev, Leonid I wrote: while triggering EIO in invalidate_inode_pages2_range() ... With this patch aio-stress sees -EIO. Actually if invalidate_inode_pages2_range() returns EIO it means that internal kernel synchronization conflict was happen.

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Benjamin LaHaise
On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: aio is not responsible for this particular synchronization. Those fixes (if we make them) should come from other places. The patch is important to get aio error handling right. I would argue that one common cause of the EIO is

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Zach Brown
I would argue that one common cause of the EIO is userland error (mmap concurrent with O_DIRECT), and EIO is the correct answer. I disagree. That means that using the pagecache to synchronize things like the proposed online defragmentation will occasionally make O_DIRECT users fail.

Re: [PATCH] aio: propogate post-EIOCBQUEUED errors to completion event

2007-02-19 Thread Chris Mason
On Mon, Feb 19, 2007 at 07:21:09PM -0500, Benjamin LaHaise wrote: On Mon, Feb 19, 2007 at 04:50:48PM -0500, Chris Mason wrote: aio is not responsible for this particular synchronization. Those fixes (if we make them) should come from other places. The patch is important to get aio error