Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-25 Thread Nick Piggin
David Chinner wrote: Only if we leave the page in the page cache. If we toss the page, the time it takes to do the I/O for the page fault is enough for the direct I/o to complete. Sure it's not an absolute guarantee, but if you want an absolute guarantee: So I guess you *could* relax it in the

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Thu, Jan 25, 2007 at 03:25:29PM +1100, Nick Piggin wrote: > David Chinner wrote: > >On Thu, Jan 25, 2007 at 01:01:09PM +1100, Nick Piggin wrote: > > > >>David Chinner wrote: > > >>>No. The only thing that will happen here is that the direct read > >>>will see _none_ of the write because the mma

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
David Chinner wrote: On Thu, Jan 25, 2007 at 01:01:09PM +1100, Nick Piggin wrote: David Chinner wrote: No. The only thing that will happen here is that the direct read will see _none_ of the write because the mmap write occurred during the DIO read to a different set of pages in memory. Ther

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Thu, Jan 25, 2007 at 01:01:09PM +1100, Nick Piggin wrote: > David Chinner wrote: > >On Thu, Jan 25, 2007 at 11:47:24AM +1100, Nick Piggin wrote: > > > >>David Chinner wrote: > >> > >>>On Thu, Jan 25, 2007 at 11:12:41AM +1100, Nick Piggin wrote: > >>But I'm just interested about DIO reads. I thin

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
David Chinner wrote: On Thu, Jan 25, 2007 at 11:47:24AM +1100, Nick Piggin wrote: David Chinner wrote: On Thu, Jan 25, 2007 at 11:12:41AM +1100, Nick Piggin wrote: ... so surely if you do a direct read followed by a buffered read, you should *not* get the same data if there has been some a

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Thu, Jan 25, 2007 at 11:47:24AM +1100, Nick Piggin wrote: > David Chinner wrote: > >On Thu, Jan 25, 2007 at 11:12:41AM +1100, Nick Piggin wrote: > > >>... so surely if you do a direct read followed by a buffered read, > >>you should *not* get the same data if there has been some activity > >>to

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
David Chinner wrote: On Thu, Jan 25, 2007 at 11:12:41AM +1100, Nick Piggin wrote: ... so surely if you do a direct read followed by a buffered read, you should *not* get the same data if there has been some activity to modify that part of the file in the meantime (whether that be a buffered or

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Thu, Jan 25, 2007 at 11:12:41AM +1100, Nick Piggin wrote: > David Chinner wrote: > >On Thu, Jan 25, 2007 at 12:43:23AM +1100, Nick Piggin wrote: > > >>And why not just leave it in the pagecache and be done with it? > > > > > >because what is in cache is then not coherent with what is on disk, >

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
David Chinner wrote: On Thu, Jan 25, 2007 at 12:43:23AM +1100, Nick Piggin wrote: And why not just leave it in the pagecache and be done with it? because what is in cache is then not coherent with what is on disk, and a direct read is supposed to read the data that is present in the file at

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
Peter Zijlstra wrote: On Thu, 2007-01-25 at 00:43 +1100, Nick Piggin wrote: Have you seen the new launder_page() a_op? called from invalidate_inode_pages2_range() It would have been nice to make that one into a more potentially useful generic callback. That can still be done when the need

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Thu, Jan 25, 2007 at 12:43:23AM +1100, Nick Piggin wrote: > Peter Zijlstra wrote: > >On Wed, 2007-01-24 at 09:37 +1100, David Chinner wrote: > > > >>With the recent changes to cancel_dirty_pages(), XFS will > >>dump warnings in the syslog because it can truncate_inode_pages() > >>on dirty mapped

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread David Chinner
On Wed, Jan 24, 2007 at 01:13:55PM +0100, Peter Zijlstra wrote: > On Wed, 2007-01-24 at 09:37 +1100, David Chinner wrote: > > With the recent changes to cancel_dirty_pages(), XFS will > > dump warnings in the syslog because it can truncate_inode_pages() > > on dirty mapped pages. > > > > I've dete

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Peter Zijlstra
On Thu, 2007-01-25 at 00:43 +1100, Nick Piggin wrote: > > Have you seen the new launder_page() a_op? called from > > invalidate_inode_pages2_range() > > It would have been nice to make that one into a more potentially > useful generic callback. That can still be done when the need arises, right?

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Nick Piggin
Peter Zijlstra wrote: On Wed, 2007-01-24 at 09:37 +1100, David Chinner wrote: With the recent changes to cancel_dirty_pages(), XFS will dump warnings in the syslog because it can truncate_inode_pages() on dirty mapped pages. I've determined that this is indeed correct behaviour for XFS as this

Re: [PATCH 1/2]: Fix BUG in cancel_dirty_pages on XFS

2007-01-24 Thread Peter Zijlstra
On Wed, 2007-01-24 at 09:37 +1100, David Chinner wrote: > With the recent changes to cancel_dirty_pages(), XFS will > dump warnings in the syslog because it can truncate_inode_pages() > on dirty mapped pages. > > I've determined that this is indeed correct behaviour for XFS > as this can happen in