[bug report] btrfs: mark block groups to copy for device-replace

[bug report] btrfs: mark block groups to copy for device-replace

[Dan Carpenter]

Hello Naohiro Aota,

The patch 0d57e73ac5ae: "btrfs: mark block groups to copy for
device-replace" from Jan 26, 2021, leads to the following static
checker warning:

	fs/btrfs/dev-replace.c:505 mark_block_group_to_copy()
	error: double unlocked '&fs_info->trans_lock' (orig line 486)

fs/btrfs/dev-replace.c
   463  static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
   464                                      struct btrfs_device *src_dev)
   465  {
   466          struct btrfs_path *path;
   467          struct btrfs_key key;
   468          struct btrfs_key found_key;
   469          struct btrfs_root *root = fs_info->dev_root;
   470          struct btrfs_dev_extent *dev_extent = NULL;
   471          struct btrfs_block_group *cache;
   472          struct btrfs_trans_handle *trans;
   473          int ret = 0;
   474          u64 chunk_offset;
   475  
   476          /* Do not use "to_copy" on non-ZONED for now */
   477          if (!btrfs_is_zoned(fs_info))
   478                  return 0;
   479  
   480          mutex_lock(&fs_info->chunk_mutex);
   481  
   482          /* Ensure we don't have pending new block group */
   483          spin_lock(&fs_info->trans_lock);
   484          while (fs_info->running_transaction &&
   485                 !list_empty(&fs_info->running_transaction->dev_update_list)) {
   486                  spin_unlock(&fs_info->trans_lock);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   487                  mutex_unlock(&fs_info->chunk_mutex);
   488                  trans = btrfs_attach_transaction(root);
   489                  if (IS_ERR(trans)) {
   490                          ret = PTR_ERR(trans);
   491                          mutex_lock(&fs_info->chunk_mutex);
   492                          if (ret == -ENOENT)
   493                                  continue;

We need to take the lock before the continue;

   494                          else
   495                                  goto unlock;
   496                  }
   497  
   498                  ret = btrfs_commit_transaction(trans);
   499                  mutex_lock(&fs_info->chunk_mutex);
   500                  if (ret)
   501                          goto unlock;
   502  
   503                  spin_lock(&fs_info->trans_lock);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   504          }
   505          spin_unlock(&fs_info->trans_lock);

Double unlock here.

   506  
   507          path = btrfs_alloc_path();
   508          if (!path) {

regards,
dan carpenter

Re: [bug report] btrfs: mark block groups to copy for device-replace

[Johannes Thumshirn]

On 03/02/2021 09:57, Dan Carpenter wrote:
> Hello Naohiro Aota,
> 
> The patch 0d57e73ac5ae: "btrfs: mark block groups to copy for
> device-replace" from Jan 26, 2021, leads to the following static
> checker warning:
> 
> 	fs/btrfs/dev-replace.c:505 mark_block_group_to_copy()
> 	error: double unlocked '&fs_info->trans_lock' (orig line 486)
> 

Fixed thanks for the report.