Re: [PATCH 12/18] Btrfs: Fix misuse of chunk mutex

2014-09-11 Thread Chris Mason


On 09/03/2014 09:35 AM, Miao Xie wrote:
 There were several problems about chunk mutex usage:
 - Lock chunk mutex when updating metadata. It would cause the nested
   deadlock because updating metadata might need allocate new chunks
   that need acquire chunk mutex. We remove chunk mutex at this case,
   because b-tree lock and other lock mechanism can help us.
 - ABBA deadlock occured between device_list_mutex and chunk_mutex.
   When we update device status, we must acquire device_list_mutex at the
   beginning, and then we might get chunk_mutex during the device status
   update because we need allocate new chunks for metadata COW. But at
   most place, we acquire chunk_mutex at first and then acquire device list
   mutex. We need change the lock order.
 - Some place we needn't acquire chunk_mutex. For example we needn't get
   chunk_mutex when we free a empty seed fs_devices structure.
 
 diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
 index 9f22398d..357f911 100644
 --- a/fs/btrfs/volumes.c
 +++ b/fs/btrfs/volumes.c
 
 @@ -2619,10 +2615,23 @@ static int btrfs_relocate_chunk(struct btrfs_root 
 *root,
   map = (struct map_lookup *)em-bdev;
  
   for (i = 0; i  map-num_stripes; i++) {
 - ret = btrfs_free_dev_extent(trans, map-stripes[i].dev,
 - map-stripes[i].physical);
 + device = map-stripes[i].dev;
 + ret = btrfs_free_dev_extent(trans, device,
 + map-stripes[i].physical,
 + dev_extent_len);
   BUG_ON(ret);

gcc is worried that dev_extent_len may be used uninitialized here.  The
BUG_ON makes it unlikely we'll notice dev_extent_len, but I set it to
zero in my version here.

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


[PATCH 12/18] Btrfs: Fix misuse of chunk mutex

2014-09-03 Thread Miao Xie
There were several problems about chunk mutex usage:
- Lock chunk mutex when updating metadata. It would cause the nested
  deadlock because updating metadata might need allocate new chunks
  that need acquire chunk mutex. We remove chunk mutex at this case,
  because b-tree lock and other lock mechanism can help us.
- ABBA deadlock occured between device_list_mutex and chunk_mutex.
  When we update device status, we must acquire device_list_mutex at the
  beginning, and then we might get chunk_mutex during the device status
  update because we need allocate new chunks for metadata COW. But at
  most place, we acquire chunk_mutex at first and then acquire device list
  mutex. We need change the lock order.
- Some place we needn't acquire chunk_mutex. For example we needn't get
  chunk_mutex when we free a empty seed fs_devices structure.

Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
 fs/btrfs/dev-replace.c |   6 +--
 fs/btrfs/extent-tree.c |   2 -
 fs/btrfs/volumes.c | 129 -
 3 files changed, 65 insertions(+), 72 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index da7ac14..aa4c828 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -510,8 +510,8 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info 
*fs_info,
WARN_ON(ret);
 
/* keep away write_all_supers() during the finishing procedure */
-   mutex_lock(root-fs_info-chunk_mutex);
mutex_lock(root-fs_info-fs_devices-device_list_mutex);
+   mutex_lock(root-fs_info-chunk_mutex);
btrfs_dev_replace_lock(dev_replace);
dev_replace-replace_state =
scrub_ret ? BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED
@@ -534,8 +534,8 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info 
*fs_info,
  src_device-devid,
  rcu_str_deref(tgt_device-name), scrub_ret);
btrfs_dev_replace_unlock(dev_replace);
-   mutex_unlock(root-fs_info-fs_devices-device_list_mutex);
mutex_unlock(root-fs_info-chunk_mutex);
+   mutex_unlock(root-fs_info-fs_devices-device_list_mutex);
if (tgt_device)
btrfs_destroy_dev_replace_tgtdev(fs_info, tgt_device);
mutex_unlock(dev_replace-lock_finishing_cancel_unmount);
@@ -589,8 +589,8 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info 
*fs_info,
 * superblock is scratched out so that it is no longer marked to
 * belong to this filesystem.
 */
-   mutex_unlock(root-fs_info-fs_devices-device_list_mutex);
mutex_unlock(root-fs_info-chunk_mutex);
+   mutex_unlock(root-fs_info-fs_devices-device_list_mutex);
 
/* write back the superblocks */
trans = btrfs_start_transaction(root, 0);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e105558..e1ad84e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9404,8 +9404,6 @@ int btrfs_remove_block_group(struct btrfs_trans_handle 
*trans,
 
memcpy(key, block_group-key, sizeof(key));
 
-   btrfs_clear_space_info_full(root-fs_info);
-
btrfs_put_block_group(block_group);
btrfs_put_block_group(block_group);
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9f22398d..357f911 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1264,7 +1264,7 @@ out:
 
 static int btrfs_free_dev_extent(struct btrfs_trans_handle *trans,
  struct btrfs_device *device,
- u64 start)
+ u64 start, u64 *dev_extent_len)
 {
int ret;
struct btrfs_path *path;
@@ -1306,13 +1306,8 @@ again:
goto out;
}
 
-   if (device-bytes_used  0) {
-   u64 len = btrfs_dev_extent_length(leaf, extent);
-   btrfs_device_set_bytes_used(device, device-bytes_used - len);
-   spin_lock(root-fs_info-free_chunk_lock);
-   root-fs_info-free_chunk_space += len;
-   spin_unlock(root-fs_info-free_chunk_lock);
-   }
+   *dev_extent_len = btrfs_dev_extent_length(leaf, extent);
+
ret = btrfs_del_item(trans, root, path);
if (ret) {
btrfs_error(root-fs_info, ret,
@@ -1521,7 +1516,6 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
key.type = BTRFS_DEV_ITEM_KEY;
key.offset = device-devid;
-   lock_chunks(root);
 
ret = btrfs_search_slot(trans, root, key, path, -1, 1);
if (ret  0)
@@ -1537,7 +1531,6 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
goto out;
 out:
btrfs_free_path(path);
-   unlock_chunks(root);
btrfs_commit_transaction(trans, root);
return ret;
 }
@@ -1726,9 +1719,7 @@ int btrfs_rm_device(struct btrfs_root *root, char 
*device_path)