[PATCH 1/2] Ext2: free memory allocated and forget buffer head when io error happens

[PATCH 1/2] Ext2: free memory allocated and forget buffer head when io error happens

[shilong wang]

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

Add a necessary check when a io error happens.
 If io error happens,free the memory has been allocated and forget buffer head.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
 fs/ext2/inode.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 6363ac6..661c816 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -495,6 +495,10 @@ static int ext2_alloc_branch(struct inode *inode,
 		 * parent to disk.
 		 */
 		bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
+		if (!bh) {
+			err = -EIO;
+			goto failed;
+		}
 		branch[n].bh = bh;
 		lock_buffer(bh);
 		memset(bh->b_data, 0, blocksize);
@@ -523,6 +527,14 @@ static int ext2_alloc_branch(struct inode *inode,
 	}
 	*blks = num;
 	return err;
+
+failed:
+	for (i = 1; i < n; i++)
+		bforget(branch[i].bh);
+	for (i = 0; i < indirect_blks; i++)
+		ext2_free_blocks(inode, new_blocks[i], 1);
+	ext2_free_blocks(inode, new_blocks[i], num);
+	return err;
 }

 /**
-- 
1.7.7.6

Re: [PATCH 1/2] Ext2: free memory allocated and forget buffer head when io error happens

[Jan Kara]

On Fri 11-01-13 03:56:45, shilong wang wrote:
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> Add a necessary check when a io error happens.
>  If io error happens,free the memory has been allocated and forget buffer head.
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> ---
>  fs/ext2/inode.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 6363ac6..661c816 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -495,6 +495,10 @@ static int ext2_alloc_branch(struct inode *inode,
>  		 * parent to disk.
>  		 */
>  		bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
> +		if (!bh) {
> +			err = -EIO;
> +			goto failed;
> +		}
  I think it should rather be ENOMEM because that's the only real reason
why sb_getblk() can fail...

>  		branch[n].bh = bh;
>  		lock_buffer(bh);
>  		memset(bh->b_data, 0, blocksize);
> @@ -523,6 +527,14 @@ static int ext2_alloc_branch(struct inode *inode,
>  	}
>  	*blks = num;
>  	return err;
> +
> +failed:
> +	for (i = 1; i < n; i++)
> +		bforget(branch[i].bh);
  I don't think we should bforget() the buffer here. They will be cleaned
up later in ext2_get_blocks().

> +	for (i = 0; i < indirect_blks; i++)
> +		ext2_free_blocks(inode, new_blocks[i], 1);
> +	ext2_free_blocks(inode, new_blocks[i], num);
> +	return err;
>  }

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/2] Ext2: free memory allocated and forget buffer head when io error happens

[Jan Kara]

On Fri 11-01-13 17:22:02, Jan Kara wrote:
> On Fri 11-01-13 03:56:45, shilong wang wrote:
> > From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> > 
> > Add a necessary check when a io error happens.
> >  If io error happens,free the memory has been allocated and forget buffer head.
> > 
> > Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> > ---
> >  fs/ext2/inode.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> > index 6363ac6..661c816 100644
> > --- a/fs/ext2/inode.c
> > +++ b/fs/ext2/inode.c
> > @@ -495,6 +495,10 @@ static int ext2_alloc_branch(struct inode *inode,
> >  		 * parent to disk.
> >  		 */
> >  		bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
> > +		if (!bh) {
> > +			err = -EIO;
> > +			goto failed;
> > +		}
>   I think it should rather be ENOMEM because that's the only real reason
> why sb_getblk() can fail...
> 
> >  		branch[n].bh = bh;
> >  		lock_buffer(bh);
> >  		memset(bh->b_data, 0, blocksize);
> > @@ -523,6 +527,14 @@ static int ext2_alloc_branch(struct inode *inode,
> >  	}
> >  	*blks = num;
> >  	return err;
> > +
> > +failed:
> > +	for (i = 1; i < n; i++)
> > +		bforget(branch[i].bh);
>   I don't think we should bforget() the buffer here. They will be cleaned
> up later in ext2_get_blocks().
  Ah, sorry, I was wrong here. The code is right. So my comment is just
about conserning the return value.

									Honza
> 
> > +	for (i = 0; i < indirect_blks; i++)
> > +		ext2_free_blocks(inode, new_blocks[i], 1);
> > +	ext2_free_blocks(inode, new_blocks[i], num);
> > +	return err;
> >  }
> 
> 									Honza
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR