Patchwork [02/12,v2] Revert "ext4: fix fsx truncate failure"

login
register
mail settings
Submitter Lukas Czerner
Date July 13, 2012, 1:19 p.m.
Message ID <1342185555-21146-2-git-send-email-lczerner@redhat.com>
Download mbox | patch
Permalink /patch/170886/
State Superseded
Headers show

Comments

Lukas Czerner - July 13, 2012, 1:19 p.m.
This reverts commit 189e868fa8fdca702eb9db9d8afc46b5cb9144c9.

This commit reintroduces the use of ext4_block_truncate_page() in ext4
truncate operation instead of ext4_discard_partial_page_buffers().

The statement in the commit description that the truncate operation only
zero block unaligned portion of the last page is not exactly right,
since truncate_pagecache_range() also zeroes and invalidate the unaligned
portion of the page. Then there is no need to zero and unmap it once more
and ext4_block_truncate_page() was doing the right job, although we
still need to update the buffer head containing the last block, which is
exactly what ext4_block_truncate_page() is doing.

This was tested on ppc64 machine with block size of 1024 bytes without
any problems.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/extents.c  |   13 ++-----------
 fs/ext4/indirect.c |   13 ++-----------
 2 files changed, 4 insertions(+), 22 deletions(-)
Theodore Ts'o - July 13, 2012, 5:42 p.m.
Hi Lukas,

This patch set has changes to the VFS, XFS, and ext4, and there are
cross dependencies between them.  Is there any way you can disentagle
the dependencies between the patches so we don't need to worry about
how these get pulled in during the merge window?

I suppose could try to get the XFS folks to sign off with my carrying
this patch in the ext4 tree, since the bulk of the changes are
ext4-related, but it is a bit of a complication.

	      	     	      	 - Ted
--
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
Christoph Hellwig - July 14, 2012, 7:45 a.m.
On Fri, Jul 13, 2012 at 01:42:09PM -0400, Theodore Ts'o wrote:
> Hi Lukas,
> 
> This patch set has changes to the VFS, XFS, and ext4, and there are
> cross dependencies between them.  Is there any way you can disentagle
> the dependencies between the patches so we don't need to worry about
> how these get pulled in during the merge window?
> 
> I suppose could try to get the XFS folks to sign off with my carrying
> this patch in the ext4 tree, since the bulk of the changes are
> ext4-related, but it is a bit of a complication.

There are no real XFS changes, just changing the calling conventions for
a generic function.  Adding this through another tree if properly
reviewed seems perfectly fine.
--
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
Lukas Czerner - July 16, 2012, 7:35 a.m.
On Fri, 13 Jul 2012, Theodore Ts'o wrote:

> Date: Fri, 13 Jul 2012 13:42:09 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
>     achender@linux.vnet.ibm.com
> Subject: Re: [PATCH 02/12 v2] Revert "ext4: fix fsx truncate failure"
> 
> Hi Lukas,
> 
> This patch set has changes to the VFS, XFS, and ext4, and there are
> cross dependencies between them.  Is there any way you can disentagle
> the dependencies between the patches so we don't need to worry about
> how these get pulled in during the merge window?
> 
> I suppose could try to get the XFS folks to sign off with my carrying
> this patch in the ext4 tree, since the bulk of the changes are
> ext4-related, but it is a bit of a complication.
> 
> 	      	     	      	 - Ted

Hi Ted,

there is no VFS change, MM is probably what you've had in mind ? So
the only reason why there are xfs, mm and tmpfs changes is that I am
changing truncate_partial_page() and a small side effect is changed
calling conventions.

We can push patches 3, 4, 5 and 6 through the mm tree. But we'll
have to make sure that it will land before ext4 changes since I am
using the new truncate_partial_page() behaviour in ext4. Will that
be possible ?

Hugh ?

-Lukas
--
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
Hugh Dickins - July 16, 2012, 9:41 p.m.
On Mon, 16 Jul 2012, Lukas Czerner wrote:
> On Fri, 13 Jul 2012, Theodore Ts'o wrote:
> > Date: Fri, 13 Jul 2012 13:42:09 -0400
> > From: Theodore Ts'o <tytso@mit.edu>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
> >     achender@linux.vnet.ibm.com
> > Subject: Re: [PATCH 02/12 v2] Revert "ext4: fix fsx truncate failure"
> > 
> > Hi Lukas,
> > 
> > This patch set has changes to the VFS, XFS, and ext4, and there are
> > cross dependencies between them.  Is there any way you can disentagle
> > the dependencies between the patches so we don't need to worry about
> > how these get pulled in during the merge window?
> > 
> > I suppose could try to get the XFS folks to sign off with my carrying
> > this patch in the ext4 tree, since the bulk of the changes are
> > ext4-related, but it is a bit of a complication.
> > 
> > 	      	     	      	 - Ted
> 
> Hi Ted,
> 
> there is no VFS change, MM is probably what you've had in mind ? So
> the only reason why there are xfs, mm and tmpfs changes is that I am
> changing truncate_partial_page() and a small side effect is changed
> calling conventions.

truncate_partial_page() is static inline to mm/truncate.c:
was that a typo, or are you changing that too and I missed it?

Sorry, but I find your changes of the infinity convention from -1 to
LLONG_MAX just gratuitous and confusing (and error prone if we ever
have to backport portions to earlier stable releases).

It grieves me enough that unmap_mapping_range() has always had quite
a different convention from truncate_inode_pages_range().  You're now
proposing to give truncate_inode_pages_range() yet another convention
different from invalidate_inode_pages2_range() and
truncate_pagecache_range()?

At cost of having to make xfs and tmpfs (well, actually it's the
tiny !SHMEM ramfs case you're changing there in 03/12) dependent
on synchronizing with your other changes?

I can see that when I converted shmem_truncate_range() (later extended
to shmem_undo_range()) to partial_start and partial_end, I did put in
an ugly couple of lines
	if (lend == -1)
		end = -1;	/* unsigned, so actually very big */
but I think it's better there than "lend == -1 ? LLONG_MAX : lend;"
ugliness spread around in the filesystems.

I don't see any upside: please, could you just drop those changes,
so that then it comes down to synchronizing ext4 with mm/truncate.c?

> 
> We can push patches 3, 4, 5 and 6 through the mm tree. But we'll
> have to make sure that it will land before ext4 changes since I am
> using the new truncate_partial_page() behaviour in ext4. Will that
> be possible ?
> 
> Hugh ?

I'd like patchs 3, 4 and 5 to vanish.  As to 6 (perhaps it won't be
6 after that!), I want to work through that one myself (I'll try this
evening): it looked over-complicated to me, and I'd rather go back to
what I worked out for partial_end in shmem_truncate_range().

Then yes, if we're all happy with the result of that, it will be best
for Ted to take even the mm/truncate.c patch into his ext4 branch,
made visible in linux-next before the merge window.

We're running out of time: I'll make every effort to get back to you
on 6 after I've tested late today.

Hugh
--
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
Lukas Czerner - July 17, 2012, 7:53 a.m.
On Mon, 16 Jul 2012, Hugh Dickins wrote:

> Date: Mon, 16 Jul 2012 14:41:41 -0700 (PDT)
> From: Hugh Dickins <hughd@google.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: 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 02/12 v2] Revert "ext4: fix fsx truncate failure"
> 
> On Mon, 16 Jul 2012, Lukas Czerner wrote:
> > On Fri, 13 Jul 2012, Theodore Ts'o wrote:
> > > Date: Fri, 13 Jul 2012 13:42:09 -0400
> > > From: Theodore Ts'o <tytso@mit.edu>
> > > To: Lukas Czerner <lczerner@redhat.com>
> > > Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
> > >     achender@linux.vnet.ibm.com
> > > Subject: Re: [PATCH 02/12 v2] Revert "ext4: fix fsx truncate failure"
> > > 
> > > Hi Lukas,
> > > 
> > > This patch set has changes to the VFS, XFS, and ext4, and there are
> > > cross dependencies between them.  Is there any way you can disentagle
> > > the dependencies between the patches so we don't need to worry about
> > > how these get pulled in during the merge window?
> > > 
> > > I suppose could try to get the XFS folks to sign off with my carrying
> > > this patch in the ext4 tree, since the bulk of the changes are
> > > ext4-related, but it is a bit of a complication.
> > > 
> > > 	      	     	      	 - Ted
> > 
> > Hi Ted,
> > 
> > there is no VFS change, MM is probably what you've had in mind ? So
> > the only reason why there are xfs, mm and tmpfs changes is that I am
> > changing truncate_partial_page() and a small side effect is changed
> > calling conventions.
> 
> truncate_partial_page() is static inline to mm/truncate.c:
> was that a typo, or are you changing that too and I missed it?

Sorry, I meant truncate_inode_pages_range().

> 
> Sorry, but I find your changes of the infinity convention from -1 to
> LLONG_MAX just gratuitous and confusing (and error prone if we ever
> have to backport portions to earlier stable releases).
> 
> It grieves me enough that unmap_mapping_range() has always had quite
> a different convention from truncate_inode_pages_range().  You're now
> proposing to give truncate_inode_pages_range() yet another convention
> different from invalidate_inode_pages2_range() and
> truncate_pagecache_range()?
> 
> At cost of having to make xfs and tmpfs (well, actually it's the
> tiny !SHMEM ramfs case you're changing there in 03/12) dependent
> on synchronizing with your other changes?
> 
> I can see that when I converted shmem_truncate_range() (later extended
> to shmem_undo_range()) to partial_start and partial_end, I did put in
> an ugly couple of lines
> 	if (lend == -1)
> 		end = -1;	/* unsigned, so actually very big */
> but I think it's better there than "lend == -1 ? LLONG_MAX : lend;"
> ugliness spread around in the filesystems.

I find the -1 convention rather confusing and the above, as you said,
quite ugly. I seemed to me much cleaner to just send what we
actually want, which is LLONG_MAX. You're right that "lend == -1 ?
LLONG_MAX : lend;" is not pretty, but it at least gives the file
systems reason to fix that and do not use -1.

It is bad enough that zero_user_segment() and friends has absolutely
weird convention where instead of "end" you actually expect "end +
1", whereas zero_user() actually uses start + size instead. Things
like that, plus the -1, makes it easy to confuse (at least for me).

> 
> I don't see any upside: please, could you just drop those changes,
> so that then it comes down to synchronizing ext4 with mm/truncate.c?

The reason of this change to teach truncate_inode_pages_range() to
handle non page aligned ranges which we then can make use of in
ext4.

> 
> > 
> > We can push patches 3, 4, 5 and 6 through the mm tree. But we'll
> > have to make sure that it will land before ext4 changes since I am
> > using the new truncate_partial_page() behaviour in ext4. Will that
> > be possible ?
> > 
> > Hugh ?
> 
> I'd like patchs 3, 4 and 5 to vanish.  As to 6 (perhaps it won't be
> 6 after that!), I want to work through that one myself (I'll try this
> evening): it looked over-complicated to me, and I'd rather go back to
> what I worked out for partial_end in shmem_truncate_range().

The code in the shmem_undo_range() is really similar, except it is
not able to handle offset of LLONG_MAX, or actually even the last
page with this offset, but truncate_inode_pages_range() obviously
does. But again, there is the confusion with the "end" being "end + 1"
which seems odd.

> 
> Then yes, if we're all happy with the result of that, it will be best
> for Ted to take even the mm/truncate.c patch into his ext4 branch,
> made visible in linux-next before the merge window.
> 
> We're running out of time: I'll make every effort to get back to you
> on 6 after I've tested late today.

Great, thanks Hugh.

-Lukas

> 
> Hugh
> 
--
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
Eric Sandeen - July 18, 2012, 7:34 p.m.
On 7/13/12 8:19 AM, Lukas Czerner wrote:
> This reverts commit 189e868fa8fdca702eb9db9d8afc46b5cb9144c9.
> 
> This commit reintroduces the use of ext4_block_truncate_page() in ext4
> truncate operation instead of ext4_discard_partial_page_buffers().

And I think commit 15291164b22a357cb211b618adfef4fa82fc0de3
jbd2: clear BH_Delay & BH_Unwritten in journal_unmap_buffer

fixed the fsx problem referenced in that commit more properly.

> The statement in the commit description that the truncate operation only
> zero block unaligned portion of the last page is not exactly right,
> since truncate_pagecache_range() also zeroes and invalidate the unaligned
> portion of the page. Then there is no need to zero and unmap it once more
> and ext4_block_truncate_page() was doing the right job, although we
> still need to update the buffer head containing the last block, which is
> exactly what ext4_block_truncate_page() is doing.
> 
> This was tested on ppc64 machine with block size of 1024 bytes without
> any problems.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  fs/ext4/extents.c  |   13 ++-----------
>  fs/ext4/indirect.c |   13 ++-----------
>  2 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 91341ec..ceab5f5 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4222,7 +4222,6 @@ void ext4_ext_truncate(struct inode *inode)
>  	struct super_block *sb = inode->i_sb;
>  	ext4_lblk_t last_block;
>  	handle_t *handle;
> -	loff_t page_len;
>  	int err = 0;
>  
>  	/*
> @@ -4239,16 +4238,8 @@ void ext4_ext_truncate(struct inode *inode)
>  	if (IS_ERR(handle))
>  		return;
>  
> -	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
> -		page_len = PAGE_CACHE_SIZE -
> -			(inode->i_size & (PAGE_CACHE_SIZE - 1));
> -
> -		err = ext4_discard_partial_page_buffers(handle,
> -			mapping, inode->i_size, page_len, 0);
> -
> -		if (err)
> -			goto out_stop;
> -	}
> +	if (inode->i_size & (sb->s_blocksize - 1))
> +		ext4_block_truncate_page(handle, mapping, inode->i_size);
>  
>  	if (ext4_orphan_add(handle, inode))
>  		goto out_stop;
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index 830e1b2..a082b30 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -1349,9 +1349,7 @@ void ext4_ind_truncate(struct inode *inode)
>  	__le32 nr = 0;
>  	int n = 0;
>  	ext4_lblk_t last_block, max_block;
> -	loff_t page_len;
>  	unsigned blocksize = inode->i_sb->s_blocksize;
> -	int err;
>  
>  	handle = start_transaction(inode);
>  	if (IS_ERR(handle))
> @@ -1362,16 +1360,9 @@ void ext4_ind_truncate(struct inode *inode)
>  	max_block = (EXT4_SB(inode->i_sb)->s_bitmap_maxbytes + blocksize-1)
>  					>> EXT4_BLOCK_SIZE_BITS(inode->i_sb);
>  
> -	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
> -		page_len = PAGE_CACHE_SIZE -
> -			(inode->i_size & (PAGE_CACHE_SIZE - 1));
> -
> -		err = ext4_discard_partial_page_buffers(handle,
> -			mapping, inode->i_size, page_len, 0);
> -
> -		if (err)
> +	if (inode->i_size & (blocksize - 1))
> +		if (ext4_block_truncate_page(handle, mapping, inode->i_size))
>  			goto out_stop;
> -	}
>  
>  	if (last_block != max_block) {
>  		n = ext4_block_to_path(inode, last_block, offsets, NULL);
> 


--
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
Lukas Czerner - July 19, 2012, 6:45 a.m.
On Wed, 18 Jul 2012, Eric Sandeen wrote:

> Date: Wed, 18 Jul 2012 14:34:47 -0500
> From: Eric Sandeen <sandeen@redhat.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, tytso@mit.edu,
>     achender@linux.vnet.ibm.com
> Subject: Re: [PATCH 02/12 v2] Revert "ext4: fix fsx truncate failure"
> 
> On 7/13/12 8:19 AM, Lukas Czerner wrote:
> > This reverts commit 189e868fa8fdca702eb9db9d8afc46b5cb9144c9.
> > 
> > This commit reintroduces the use of ext4_block_truncate_page() in ext4
> > truncate operation instead of ext4_discard_partial_page_buffers().
> 
> And I think commit 15291164b22a357cb211b618adfef4fa82fc0de3
> jbd2: clear BH_Delay & BH_Unwritten in journal_unmap_buffer
> 
> fixed the fsx problem referenced in that commit more properly.

You're probably right, I'll test that with your test case and add it
to the description.

Thanks!
-Lukas

> 
> > The statement in the commit description that the truncate operation only
> > zero block unaligned portion of the last page is not exactly right,
> > since truncate_pagecache_range() also zeroes and invalidate the unaligned
> > portion of the page. Then there is no need to zero and unmap it once more
> > and ext4_block_truncate_page() was doing the right job, although we
> > still need to update the buffer head containing the last block, which is
> > exactly what ext4_block_truncate_page() is doing.
> > 
> > This was tested on ppc64 machine with block size of 1024 bytes without
> > any problems.
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> >  fs/ext4/extents.c  |   13 ++-----------
> >  fs/ext4/indirect.c |   13 ++-----------
> >  2 files changed, 4 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > index 91341ec..ceab5f5 100644
> > --- a/fs/ext4/extents.c
> > +++ b/fs/ext4/extents.c
> > @@ -4222,7 +4222,6 @@ void ext4_ext_truncate(struct inode *inode)
> >  	struct super_block *sb = inode->i_sb;
> >  	ext4_lblk_t last_block;
> >  	handle_t *handle;
> > -	loff_t page_len;
> >  	int err = 0;
> >  
> >  	/*
> > @@ -4239,16 +4238,8 @@ void ext4_ext_truncate(struct inode *inode)
> >  	if (IS_ERR(handle))
> >  		return;
> >  
> > -	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
> > -		page_len = PAGE_CACHE_SIZE -
> > -			(inode->i_size & (PAGE_CACHE_SIZE - 1));
> > -
> > -		err = ext4_discard_partial_page_buffers(handle,
> > -			mapping, inode->i_size, page_len, 0);
> > -
> > -		if (err)
> > -			goto out_stop;
> > -	}
> > +	if (inode->i_size & (sb->s_blocksize - 1))
> > +		ext4_block_truncate_page(handle, mapping, inode->i_size);
> >  
> >  	if (ext4_orphan_add(handle, inode))
> >  		goto out_stop;
> > diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> > index 830e1b2..a082b30 100644
> > --- a/fs/ext4/indirect.c
> > +++ b/fs/ext4/indirect.c
> > @@ -1349,9 +1349,7 @@ void ext4_ind_truncate(struct inode *inode)
> >  	__le32 nr = 0;
> >  	int n = 0;
> >  	ext4_lblk_t last_block, max_block;
> > -	loff_t page_len;
> >  	unsigned blocksize = inode->i_sb->s_blocksize;
> > -	int err;
> >  
> >  	handle = start_transaction(inode);
> >  	if (IS_ERR(handle))
> > @@ -1362,16 +1360,9 @@ void ext4_ind_truncate(struct inode *inode)
> >  	max_block = (EXT4_SB(inode->i_sb)->s_bitmap_maxbytes + blocksize-1)
> >  					>> EXT4_BLOCK_SIZE_BITS(inode->i_sb);
> >  
> > -	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
> > -		page_len = PAGE_CACHE_SIZE -
> > -			(inode->i_size & (PAGE_CACHE_SIZE - 1));
> > -
> > -		err = ext4_discard_partial_page_buffers(handle,
> > -			mapping, inode->i_size, page_len, 0);
> > -
> > -		if (err)
> > +	if (inode->i_size & (blocksize - 1))
> > +		if (ext4_block_truncate_page(handle, mapping, inode->i_size))
> >  			goto out_stop;
> > -	}
> >  
> >  	if (last_block != max_block) {
> >  		n = ext4_block_to_path(inode, last_block, offsets, NULL);
> > 
> 
> 
> 
--
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

Patch

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 91341ec..ceab5f5 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4222,7 +4222,6 @@  void ext4_ext_truncate(struct inode *inode)
 	struct super_block *sb = inode->i_sb;
 	ext4_lblk_t last_block;
 	handle_t *handle;
-	loff_t page_len;
 	int err = 0;
 
 	/*
@@ -4239,16 +4238,8 @@  void ext4_ext_truncate(struct inode *inode)
 	if (IS_ERR(handle))
 		return;
 
-	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
-		page_len = PAGE_CACHE_SIZE -
-			(inode->i_size & (PAGE_CACHE_SIZE - 1));
-
-		err = ext4_discard_partial_page_buffers(handle,
-			mapping, inode->i_size, page_len, 0);
-
-		if (err)
-			goto out_stop;
-	}
+	if (inode->i_size & (sb->s_blocksize - 1))
+		ext4_block_truncate_page(handle, mapping, inode->i_size);
 
 	if (ext4_orphan_add(handle, inode))
 		goto out_stop;
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 830e1b2..a082b30 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -1349,9 +1349,7 @@  void ext4_ind_truncate(struct inode *inode)
 	__le32 nr = 0;
 	int n = 0;
 	ext4_lblk_t last_block, max_block;
-	loff_t page_len;
 	unsigned blocksize = inode->i_sb->s_blocksize;
-	int err;
 
 	handle = start_transaction(inode);
 	if (IS_ERR(handle))
@@ -1362,16 +1360,9 @@  void ext4_ind_truncate(struct inode *inode)
 	max_block = (EXT4_SB(inode->i_sb)->s_bitmap_maxbytes + blocksize-1)
 					>> EXT4_BLOCK_SIZE_BITS(inode->i_sb);
 
-	if (inode->i_size % PAGE_CACHE_SIZE != 0) {
-		page_len = PAGE_CACHE_SIZE -
-			(inode->i_size & (PAGE_CACHE_SIZE - 1));
-
-		err = ext4_discard_partial_page_buffers(handle,
-			mapping, inode->i_size, page_len, 0);
-
-		if (err)
+	if (inode->i_size & (blocksize - 1))
+		if (ext4_block_truncate_page(handle, mapping, inode->i_size))
 			goto out_stop;
-	}
 
 	if (last_block != max_block) {
 		n = ext4_block_to_path(inode, last_block, offsets, NULL);