diff mbox

[4/4] ext4: don't keep using page if inline conversion fails

Message ID 20140911002845.10109.10558.stgit@birch.djwong.org
State Accepted, archived
Headers show

Commit Message

Darrick Wong Sept. 11, 2014, 12:28 a.m. UTC
If inline->extent conversion fails (most probably due to ENOSPC) and
we release the temporary page that we allocated to transfer the file
contents, don't keep using the page pointer after releasing the page.
This occasionally leads to complaints about evicting locked pages or
hangs when blocksize > pagesize, because it's possible for the page to
get reallocated elsewhere in the meantime.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Cc: Tao Ma <tm@tao.ma>
---
 fs/ext4/inline.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)



--
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

Comments

Jan Kara Sept. 11, 2014, 1:17 p.m. UTC | #1
On Wed 10-09-14 17:28:45, Darrick J. Wong wrote:
> If inline->extent conversion fails (most probably due to ENOSPC) and
> we release the temporary page that we allocated to transfer the file
> contents, don't keep using the page pointer after releasing the page.
> This occasionally leads to complaints about evicting locked pages or
> hangs when blocksize > pagesize, because it's possible for the page to
> get reallocated elsewhere in the meantime.
  Good catch! You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Cc: Tao Ma <tm@tao.ma>
> ---
>  fs/ext4/inline.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index bea662b..378aadf 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -594,6 +594,7 @@ retry:
>  	if (ret) {
>  		unlock_page(page);
>  		page_cache_release(page);
> +		page = NULL;
>  		ext4_orphan_add(handle, inode);
>  		up_write(&EXT4_I(inode)->xattr_sem);
>  		sem_held = 0;
> @@ -613,7 +614,8 @@ retry:
>  	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;
>  
> -	block_commit_write(page, from, to);
> +	if (page)
> +		block_commit_write(page, from, to);
>  out:
>  	if (page) {
>  		unlock_page(page);
> 
> --
> 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
Theodore Ts'o Sept. 11, 2014, 3:46 p.m. UTC | #2
On Thu, Sep 11, 2014 at 03:17:53PM +0200, Jan Kara wrote:
> On Wed 10-09-14 17:28:45, Darrick J. Wong wrote:
> > If inline->extent conversion fails (most probably due to ENOSPC) and
> > we release the temporary page that we allocated to transfer the file
> > contents, don't keep using the page pointer after releasing the page.
> > This occasionally leads to complaints about evicting locked pages or
> > hangs when blocksize > pagesize, because it's possible for the page to
> > get reallocated elsewhere in the meantime.
>   Good catch! You can add:
> Reviewed-by: Jan Kara <jack@suse.cz>

Applied, thanks.

					- 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
diff mbox

Patch

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index bea662b..378aadf 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -594,6 +594,7 @@  retry:
 	if (ret) {
 		unlock_page(page);
 		page_cache_release(page);
+		page = NULL;
 		ext4_orphan_add(handle, inode);
 		up_write(&EXT4_I(inode)->xattr_sem);
 		sem_held = 0;
@@ -613,7 +614,8 @@  retry:
 	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
 		goto retry;
 
-	block_commit_write(page, from, to);
+	if (page)
+		block_commit_write(page, from, to);
 out:
 	if (page) {
 		unlock_page(page);