| Message ID | 20220505070825.1218337-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs: Use btrfs_try_lock_balance in btrfs_ioctl_balance | expand |
On Thu, May 05, 2022 at 10:08:25AM +0300, Nikolay Borisov wrote: > This eliminates 2 labels and makes the code generally more streamlined. > Also rename the 'out_bargs' label to 'out_unlock' since bargs is going > to be freed under the 'out' label. This also fixes a memory leak since > bargs wasn't correctly freed in one of the condition which are now moved > in btrfs_try_lock_balance. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > > V2: > * Removed extra call to kfree(bargs) which resulted in a double free Where does this patch apply to? It's using btrfs_try_lock_balance but that does not exist in code nor in the patch.
On 9.05.22 г. 22:51 ч., David Sterba wrote: > On Thu, May 05, 2022 at 10:08:25AM +0300, Nikolay Borisov wrote: >> This eliminates 2 labels and makes the code generally more streamlined. >> Also rename the 'out_bargs' label to 'out_unlock' since bargs is going >> to be freed under the 'out' label. This also fixes a memory leak since >> bargs wasn't correctly freed in one of the condition which are now moved >> in btrfs_try_lock_balance. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> Reported-by: kernel test robot <lkp@intel.com> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> >> V2: >> * Removed extra call to kfree(bargs) which resulted in a double free > > Where does this patch apply to? It's using btrfs_try_lock_balance but that > does not exist in code nor in the patch. > Well this is v2 2/2 so it's part of the same series just an update to the 2nd patch.
On Tue, May 10, 2022 at 08:42:37AM +0300, Nikolay Borisov wrote: > > > On 9.05.22 г. 22:51 ч., David Sterba wrote: > > On Thu, May 05, 2022 at 10:08:25AM +0300, Nikolay Borisov wrote: > >> This eliminates 2 labels and makes the code generally more streamlined. > >> Also rename the 'out_bargs' label to 'out_unlock' since bargs is going > >> to be freed under the 'out' label. This also fixes a memory leak since > >> bargs wasn't correctly freed in one of the condition which are now moved > >> in btrfs_try_lock_balance. > >> > >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > >> Reported-by: kernel test robot <lkp@intel.com> > >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > >> --- > >> > >> V2: > >> * Removed extra call to kfree(bargs) which resulted in a double free > > > > Where does this patch apply to? It's using btrfs_try_lock_balance but that > > does not exist in code nor in the patch. > > Well this is v2 2/2 so it's part of the same series just an update to > the 2nd patch. Ah sorry, I had moved the thread to archive and then this patch did not thread in properly. Added to misc-next, thanks.
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 8e0cc17d5f81..9ac07eb7531c 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -4406,7 +4406,7 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg) struct btrfs_fs_info *fs_info = root->fs_info; struct btrfs_ioctl_balance_args *bargs; struct btrfs_balance_control *bctl; - bool need_unlock; /* for mut. excl. ops lock */ + bool need_unlock = true; /* for mut. excl. ops lock */ int ret; if (!capable(CAP_SYS_ADMIN)) @@ -4423,53 +4423,12 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg) goto out; } -again: - if (btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) { - mutex_lock(&fs_info->balance_mutex); - need_unlock = true; - goto locked; - } - - /* - * mut. excl. ops lock is locked. Three possibilities: - * (1) some other op is running - * (2) balance is running - * (3) balance is paused -- special case (think resume) - */ - mutex_lock(&fs_info->balance_mutex); - if (fs_info->balance_ctl) { - /* this is either (2) or (3) */ - if (!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) { - mutex_unlock(&fs_info->balance_mutex); - /* - * Lock released to allow other waiters to continue, - * we'll reexamine the status again. - */ - mutex_lock(&fs_info->balance_mutex); - - if (fs_info->balance_ctl && - !test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) { - /* this is (3) */ - need_unlock = false; - goto locked; - } - - mutex_unlock(&fs_info->balance_mutex); - goto again; - } else { - /* this is (2) */ - mutex_unlock(&fs_info->balance_mutex); - ret = -EINPROGRESS; - goto out; - } - } else { - /* this is (1) */ - mutex_unlock(&fs_info->balance_mutex); - ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS; + ret = btrfs_try_lock_balance(fs_info, &need_unlock); + if (ret) goto out; - } -locked: + lockdep_assert_held(&fs_info->balance_mutex); + if (bargs->flags & BTRFS_BALANCE_RESUME) { if (!fs_info->balance_ctl) { ret = -ENOTCONN;
This eliminates 2 labels and makes the code generally more streamlined. Also rename the 'out_bargs' label to 'out_unlock' since bargs is going to be freed under the 'out' label. This also fixes a memory leak since bargs wasn't correctly freed in one of the condition which are now moved in btrfs_try_lock_balance. Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> --- V2: * Removed extra call to kfree(bargs) which resulted in a double free fs/btrfs/ioctl.c | 51 +++++------------------------------------------- 1 file changed, 5 insertions(+), 46 deletions(-) -- 2.25.1