Re: [PATCH 06/12 v2] mm: teach truncate_inode_pages_range() to hadnle non page aligned ranges

[Dave Chinner <dchinner@redhat.com>]

On Thu, Jul 19, 2012 at 09:15:09AM +0200, Lukáš Czerner wrote:
> On Wed, 18 Jul 2012, Hugh Dickins wrote:
> 
> > Date: Wed, 18 Jul 2012 12:36:39 -0700 (PDT)
> > From: Hugh Dickins <hughd@google.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: Christoph Hellwig <hch@infradead.org>,
> >     Andrew Morton <akpm@linux-foundation.org>, Theodore Ts'o <tytso@mit.edu>,
> >     Dave Chinner <dchinner@redhat.com>, linux-ext4@vger.kernel.org,
> >     linux-fsdevel@vger.kernel.org, achender@linux.vnet.ibm.com
> > Subject: Re: [PATCH 06/12 v2] mm: teach truncate_inode_pages_range() to hadnle
> >      non page aligned ranges
> > 
> > On Wed, 18 Jul 2012, Lukas Czerner wrote:
> > > On Tue, 17 Jul 2012, Lukas Czerner wrote:
> > > > 
> > > > My bad, it definitely is not safe without the end offset argument in
> > > > invalidatepage() aops ..sigh..
> > > 
> > > So what about having new aop invalidatepage_range and using that in
> > > the truncate_inode_pages_range(). We can still BUG_ON if the file
> > > system register invalidatepage, but not invalidatepage_range,
> > > when the range to truncate is not page aligned at the end.
> > 
> > I had some trouble parsing what you wrote, and have slightly adjusted
> > it (mainly adding a comma) to fit my understanding: shout at me if I'm
> > misrepresenting you!
> > 
> > Yes, I think that's what has to be done.  It's irritating to have two
> > methods doing the same job, but not nearly so irritating as having to
> > change core and all filesystems at the same time.  Then at some future
> > date there can be a cleanup to remove the old invalidatepage method.
> 
> Agreed!
> 
> > 
> > > 
> > > I am sure more file system than just ext4 can take advantage of
> > > this. Currently only ext4, xfs and ocfs2 support punch hole and I
> > > think that all of them can use truncate_inode_pages_range() which
> > > handles unaligned ranges.
> > 
> > I expect that they can, but I'm far from sure of it: each filesystem
> > will have its own needs and difficulties, which might delay them from
> > a quick switchover to invalidatepage_range.
> > 
> > > 
> > > Currently ext4 has it's own overcomplicated method of freeing and
> > > zeroing unaligned ranges.
> > 
> > You're best placed to judge if its overcomplicated, I've not looked.
> > 
> > > Xfs seems just truncate the whole file and
> > 
> > I doubt that can be the case: how would it ever pass testing with
> > the hole-punching fsx if so?  But it is the case that xfs unmaps
> > all the pages from hole onwards, in the exceptional case where the
> > punched file is currently mmap'ed into userspace; and that is wrong,
> > and will get fixed, but it's not a huge big deal meanwhile.  (But it
> > does suggest that hole-punching is more difficult to get completely
> > right than people think at first.)
> 
> Ok, maybe I did not express myself very well, sorry. I meant to say
> that xfs will unmap all mapped pages in the file from start of the
> hole to the end of the file.

It might do that right now, but that's no guarantee that we won't
change it in future. Indeed, we've been considering changing all the
toss/inval page calls to just the required range for a few years,
but never got around to doing it because of we never really
understood how the VM would handle it....

Likewise, those wrappers in fs/xfs/xfs_fs_subr.c need to go away,and
we've been considering that for just as long. It's never happened
because of the above.

If the VM can handle ranged toss/inval regions correctly, then we
can make those changes without concerns of introducing data integrity
regressions....

Cheers,

Dave.
-- 
Dave Chinner
dchinner@redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html