On Fri, Apr 08, 2011 at 03:37:15PM +0200, David Sterba wrote: > Hi, > > sorry for separate mail, just noticed a kfree inside a spinlock below > > On Thu, Apr 07, 2011 at 06:06:08PM +0100, Hugo Mills wrote: > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > @@ -2115,10 +2162,20 @@ int btrfs_balance(struct btrfs_root *dev_root) > > found_key.offset); > > BUG_ON(ret && ret != -ENOSPC); > > key.offset = found_key.offset - 1; > > + spin_lock(&dev_root->fs_info->balance_info_lock); > > + bal_info->completed++; > > + spin_unlock(&dev_root->fs_info->balance_info_lock); > > + printk(KERN_INFO "btrfs: balance: %llu/%llu block groups > > completed\n", > > + bal_info->completed, bal_info->expected); > > } > > ret = 0; > > error: > > btrfs_free_path(path); > > + spin_lock(&dev_root->fs_info->balance_info_lock); > > + kfree(dev_root->fs_info->balance_info); > ^^^^^ > move it out of the spinlocked section
From IRC: <josef> darkling: you can safely kfree inside of a spinlock, dont worry about that <kdave> josef: the kfree inside spinlock seem a bit heavy to me, if you were referring to the mail <josef> kdave: kfree just inc's a stat and puts the object in the free array, so its not all that heavy <josef> not to mention this isn't in a fastpath, if it were a fastpath i'd be more worried about it I'm with josef on this -- this is a one-off action at the end of a process that's likely to take anything up to several days, so it's about as far from being a fastpath as you can get. > > + dev_root->fs_info->balance_info = NULL; > > + spin_unlock(&dev_root->fs_info->balance_info_lock); > > +error_no_status: > > mutex_unlock(&dev_root->fs_info->volume_mutex); > > return ret; > > } Hugo. -- === Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk === PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk --- Questions are a burden, and answers a prison for oneself. ---
signature.asc
Description: Digital signature
