Re: Review: fix mapping invalidation callouts

[David Chinner <dgc@sgi.com>]

On Thu, Jan 11, 2007 at 05:49:58PM +1100, David Chinner wrote:
> On Wed, Jan 10, 2007 at 08:39:33AM +0000, Lachlan McIlroy wrote:
> > David Chinner wrote:
> > >>+#define XFS_OFF_TO_PCSIZE(off)	\
> > >>+	(((off) + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT)
> > >
> > >
> > >I don't think this is right.
> > >
> > >Assuming 4k page size, first = 2k, last = 6k will result in
> > >invalidating page indexes 1 and 2 i.e. offset 4k -> 12k. In fact,
> > >we want to invalidate pages 0 and 1.
> > >
> > >IOWs, I think it should be:
> > >
> > >+#define XFS_OFF_TO_PCINDEX(off)	((off) >> PAGE_CACHE_SHIFT)
> > >
> > >Comments?
> > >
> > 
> > Makes sense to me.
> 
> Yeah, you'd think so. The first xfsqa run I do -after- checking it in
> (been running for 24 hours) I get a stack dump with the warning
> in cancel_dirty_page(), so clearly this isn't right either. I'm
> not sure WTF is going on here.

Of course, I just realised that this is 2.6.19 that I'm testing
on (fmeh) and so the code is different - cancel-dirty_page()
doesn't exist in this tree, and the warning is coming from
invalidate_inode_pages2_range() because invalidate_complete_page2()
is returning an error for some reason.....

Looks like it's a partial page truncation problem.
invalidate_inode_pages2_range() fails on partial page truncation
when part of the page (i.e. a bufferhead) is dirty.

This looks like a _big_ mess.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group