On Thu, Jul 19, 2012 at 09:15:09AM +0200, Lukáš Czerner wrote:

On Wed, 18 Jul 2012, Hugh Dickins wrote:

quoted

Date: Wed, 18 Jul 2012 12:36:39 -0700 (PDT)
From: Hugh Dickins <hughd@google.com>
To: Lukas Czerner <redacted>
Cc: Christoph Hellwig <hch@infradead.org>,
    Andrew Morton [off-list ref], Theodore Ts'o [off-list ref],
    Dave Chinner [off-list ref], 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:

quoted

On Tue, 17 Jul 2012, Lukas Czerner wrote:

quoted

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!

quoted

quoted

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.

quoted

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.

quoted

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