Re: [PATCH] Btrfs: setup blank root and fs_info for mount time

[Josef Bacik <josef@redhat.com>]

On Mon, Nov 22, 2010 at 09:49:33AM +0800, Ian Kent wrote:
> On Mon, 2010-11-22 at 09:25 +0800, Li Zefan wrote:
> > > +	/*
> > > +	 * Setup a dummy root and fs_info for test/set super.  This is because
> > > +	 * we don't actually fill this stuff out until open_ctree, but we need
> > > +	 * it for searching for existing supers, so this lets us do that and
> > > +	 * then open_ctree will properly initialize everything later.
> > > +	 */
> > > +	fs_info = kzalloc(sizeof(struct btrfs_fs_info), GFP_NOFS);
> > > +	tree_root = kzalloc(sizeof(struct btrfs_root), GFP_NOFS);
> > > +	if (!fs_info || !tree_root) {
> > > +		kfree(fs_info);
> > > +		kfree(tree_root);
> > 
> > The above 2 kfree() calls are redundant.
> 
> That's what I thought when I first looked at it but what happens when
> fs_info != NULL and tree_root == NULL.
>

When i do goto error_close_devices; i do the kfree's there too, my bad, I'll fix
it up.
 
> Although I do wonder if doing two successive allocation requests and
> then checking both is a good idea. If memory is low there may be a bunch
> of page scanning to try and free memory and if it eventually fails that
> same process probably would happen all over again on the second call.
> I'm not sure though.
>

Sure but how often is that going to happen :)
 
> > 
> > And error should be set to -ENOMEM.
> 
> Ah, yes .. I missed that.
>

Yup me too.  Thanks for the review guys,

Josef