[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 7/7] btrfs: don't BUG_ON allocation errors in btrfs_drop_snapshot



On Fri, Jul 22, 2011 at 09:45:19AM +0900, Tsutomu Itoh wrote:
> (2011/07/22 4:48), Mark Fasheh wrote:
> > In addition to properly handling allocation failure from btrfs_alloc_path, I
> > also fixed up the kzalloc error handling code immediately below it.
> > 
> > Signed-off-by: Mark Fasheh <mfasheh@xxxxxxxx>
> > ---
> >  fs/btrfs/extent-tree.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index ff339b2..4cf5257 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -6271,10 +6271,14 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
> >  	int level;
> >  
> >  	path = btrfs_alloc_path();
> > -	BUG_ON(!path);
> > +	if (!path)
> > +		return -ENOMEM;
> >  
> >  	wc = kzalloc(sizeof(*wc), GFP_NOFS);
> > -	BUG_ON(!wc);
> > +	if (!wc) {
> > +		btrfs_free_path(path);
> > +		return -ENOMEM;
> > +	}
> >  
> >  	trans = btrfs_start_transaction(tree_root, 0);
> >  	BUG_ON(IS_ERR(trans));
> 
> Currently, callers of btrfs_drop_snapshot() ignore the return code.
> But btrfs_drop_snapshot() detects the error by BUG_ON.
> 
> The caller still ignore the return code though your modification returns
> the error code to the caller. 
> So, we can not detect error. I don't think that it is good.

IMHO, this is a seperate issue that btrfs_drop_snapshot() has even without
my patch. You can see in the code that it might return any number of errors,
all of which get ignored by callers. So my patch is cleaning up some of the
BUG_ON() usage, but not really solving the 2nd problem of ignored return
codes. Of course that was on purpose as I like to fix one problem per patch
if possible and practicle.
	--Mark

--
Mark Fasheh
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]

Add to Google Powered by Linux