On Wed, 18 Jul 2012, Hugh Dickins wrote: > Date: Wed, 18 Jul 2012 12:36:39 -0700 (PDT) > From: Hugh Dickins <hughd@xxxxxxxxxx> > To: Lukas Czerner <lczerner@xxxxxxxxxx> > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, > Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Theodore Ts'o <tytso@xxxxxxx>, > Dave Chinner <dchinner@xxxxxxxxxx>, linux-ext4@xxxxxxxxxxxxxxx, > linux-fsdevel@xxxxxxxxxxxxxxx, achender@xxxxxxxxxxxxxxxxxx > 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. > > > there seems to be a bug in ocfs2 where we can hit BUG_ON when the > > cluster size < page size. > > > > What do you reckon ? > > I agree that you need invalidatepage_range for truncate_inode_page_range > to drop its end alignment restriction. But now that we have to add a > method, I think it would be more convincing justification to have two > filesystems converted to make use of it, than just the one ext4. Ok, I'll do this and try to see what I can do with some other file systems as well. Thanks! -Lukas > > Hugh > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html