From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 1/2] Ext2: free memory allocated and forget buffer head when io error happens Date: Fri, 11 Jan 2013 17:25:58 +0100 Message-ID: <20130111162558.GC16923@quack.suse.cz> References: <20130111162202.GB16923@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org To: shilong wang Return-path: Received: from cantor2.suse.de ([195.135.220.15]:41076 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754844Ab3AKQ0A (ORCPT ); Fri, 11 Jan 2013 11:26:00 -0500 Content-Disposition: inline In-Reply-To: <20130111162202.GB16923@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > > > > 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 > > --- > > 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 > 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 SUSE Labs, CR