Btrfs lockdep and locking cleanups
David Sterba
maintainercover Btrfs lockdep and locking cleanups
A few cleanups related to lockdep and locking. The btrfs_show_devname is a functional change, uses the same mechanism as btrfs_ioctl_dev_info or btrfs_ioctl_fs_info so that's not a new thing.
David Sterba (6):
btrfs: use lockdep_assert_held for spinlocks
btrfs: use lockdep_assert_held for mutexes
btrfs: update barrier in should_cow_block
btrfs: use RCU in btrfs_show_devname for device list traversal
btrfs: remove stale comments about fs_mutex
btrfs: split dev-replace locking helpers for read and write
fs/btrfs/acl.c | 8 ----- fs/btrfs/ctree.c | 4 +-- fs/btrfs/delayed-ref.c | 6 ++-- fs/btrfs/dev-replace.c | 98 +++++++++++++++++++++++++------------------------- fs/btrfs/dev-replace.h | 6 ++-- fs/btrfs/extent-tree.c | 2 +- fs/btrfs/qgroup.c | 2 +- fs/btrfs/reada.c | 10 +++--- fs/btrfs/scrub.c | 18 +++++----- fs/btrfs/super.c | 15 +++++--- fs/btrfs/volumes.c | 28 +++++++-------- 11 files changed, 98 insertions(+), 99 deletions(-)
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 1/6 btrfs: use lockdep_assert_held for spinlocks
Using lockdep_assert_held is preferred, replace assert_spin_locked.
--- fs/btrfs/delayed-ref.c | 6 +++--- fs/btrfs/qgroup.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
fs/btrfs/delayed-ref.c +3 -3
@@ -216,7 +216,7 @@ int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans,struct btrfs_delayed_ref_root *delayed_refs; delayed_refs = &trans->transaction->delayed_refs; -assert_spin_locked (&delayed_refs->lock); +lockdep_assert_held (&delayed_refs->lock);if (mutex_trylock (&head->mutex))return 0 ;
@@ -239,7 +239,7 @@ static inline void drop_delayed_ref(struct btrfs_trans_handle *trans,struct btrfs_delayed_ref_head *head, struct btrfs_delayed_ref_node *ref) { - assert_spin_locked(&head->lock); + lockdep_assert_held(&head->lock);rb_erase (&ref->ref_node, &head->ref_tree);RB_CLEAR_NODE (&ref->ref_node);if (!list_empty (&ref->add_list))
@@ -307,7 +307,7 @@ void btrfs_merge_delayed_refs(struct btrfs_trans_handle *trans,struct rb_node *node;u64 seq =0 ; -assert_spin_locked (&head->lock); +lockdep_assert_held (&head->lock);if (RB_EMPTY_ROOT (&head->ref_tree))return ;
fs/btrfs/qgroup.c +1 -1
@@ -1414,7 +1414,7 @@ int btrfs_qgroup_trace_extent_nolock(struct btrfs_fs_info *fs_info,struct btrfs_qgroup_extent_record *entry;u64 bytenr = record->bytenr; -assert_spin_locked (&delayed_refs->lock); +lockdep_assert_held (&delayed_refs->lock);trace_btrfs_qgroup_trace_extent (fs_info, record);while (*p) {
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 2/6 btrfs: use lockdep_assert_held for mutexes
In reply to David SterbaUsing lockdep_assert_held is preferred, replace mutex_is_locked.
--- fs/btrfs/extent-tree.c | 2 +- fs/btrfs/scrub.c | 4 ++-- fs/btrfs/volumes.c | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-)
fs/btrfs/extent-tree.c +1 -1
@@ -4542,7 +4542,7 @@ void check_system_chunk(struct btrfs_trans_handle *trans,* Needed because we can end up allocating a system chunk and for an * atomic and race free space reservation in the chunk block reserve. */ -ASSERT (mutex_is_locked (&fs_info->chunk_mutex)); +lockdep_assert_held (&fs_info->chunk_mutex); info =__find_space_info (fs_info,BTRFS_BLOCK_GROUP_SYSTEM );spin_lock (&info->lock);
fs/btrfs/scrub.c +2 -2
@@ -371,7 +371,7 @@ static struct full_stripe_lock *insert_full_stripe_lock(struct full_stripe_lock *entry;struct full_stripe_lock *ret; -WARN_ON (!mutex_is_locked (&locks_root->lock)); +lockdep_assert_held (&locks_root->lock); p = &locks_root->root.rb_node;while (*p) {
@@ -413,7 +413,7 @@ static struct full_stripe_lock *search_full_stripe_lock(struct rb_node *node;struct full_stripe_lock *entry; -WARN_ON (!mutex_is_locked (&locks_root->lock)); +lockdep_assert_held (&locks_root->lock); node = locks_root->root.rb_node;while (node) {
fs/btrfs/volumes.c +5 -5
@@ -2047,7 +2047,7 @@ void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_fs_info *fs_info, {struct btrfs_fs_devices *fs_devices; -WARN_ON (!mutex_is_locked (&fs_info->fs_devices->device_list_mutex)); +lockdep_assert_held (&fs_info->fs_devices->device_list_mutex);/* * in case of fs with no seed, srcdev->fs_devices will point
@@ -2237,7 +2237,7 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)struct btrfs_device *device;u64 super_flags; -BUG_ON (!mutex_is_locked (&uuid_mutex)); +lockdep_assert_held (&uuid_mutex);if (!fs_devices->seeding)return -EINVAL ;
@@ -2984,7 +2984,7 @@ static int btrfs_relocate_chunk(struct btrfs_fs_info *fs_info, u64 chunk_offset)* we release the path used to search the chunk/dev tree and before * the current task acquires this mutex and calls us. */ -ASSERT (mutex_is_locked (&fs_info->delete_unused_bgs_mutex)); +lockdep_assert_held (&fs_info->delete_unused_bgs_mutex); ret =btrfs_can_relocate (fs_info, chunk_offset);if (ret)
@@ -5068,7 +5068,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, {u64 chunk_offset; -ASSERT (mutex_is_locked (&fs_info->chunk_mutex)); +lockdep_assert_held (&fs_info->chunk_mutex); chunk_offset =find_next_chunk (fs_info);return __btrfs_alloc_chunk (trans, chunk_offset, type); }
@@ -6618,7 +6618,7 @@ static struct btrfs_fs_devices *open_seed_devices(struct btrfs_fs_info *fs_info,struct btrfs_fs_devices *fs_devices;int ret; -BUG_ON (!mutex_is_locked (&uuid_mutex)); +lockdep_assert_held (&uuid_mutex);ASSERT (fsid); fs_devices = fs_info->fs_devices->seed;
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 3/6 btrfs: update barrier in should_cow_block
In reply to David SterbaOnce there was a simple int force_cow that was used with the plain barriers, and then converted to a bit, so we should use the appropriate barrier helper.
Other variables in the complex if condition do not depend on a barrier, so we should be fine in case the atomic barrier becomes a no-op.
--- fs/btrfs/ctree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/btrfs/ctree.c +2 -2
@@ -1502,8 +1502,8 @@ static inline int should_cow_block(struct btrfs_trans_handle *trans,if (btrfs_is_testing (root->fs_info))return 0 ; -/* ensure we can see the force_cow */ -smp_rmb (); +/* Ensure we can see the FORCE_COW bit */ +smp_mb__before_atomic ();/* * We do not need to cow a block if
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 4/6 btrfs: use RCU in btrfs_show_devname for device list traversal
In reply to David SterbaThe show_devname callback is used to print device name in /proc/self/mounts, we need to traverse the device list consistently and read the name that's copied to a seq buffer so we don't need further locking.
If the first device is being deleted at the same time, the RCU will allow us to read the device name, though it will become stale right after the RCU protection ends. This is unavoidable and the user can expect that the device will disappear from the filesystem's list at some point.
The device_list_mutex was pretty heavy as it is used eg. for writing superblock and a few other IO related contexts. This can stall any application that reads the proc file for no reason.
--- fs/btrfs/super.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
fs/btrfs/super.c +10 -5
@@ -2290,11 +2290,18 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root)struct list_head *head;struct rcu_string *name; -mutex_lock (&fs_info->fs_devices->device_list_mutex); +/* +* Lightweight locking of the devices. We should not need +* device_list_mutex here as we only read the device data and the list +* is protected by RCU. Even if a device is deleted during the list +* traversals, we'll get valid data, the freeing callback will wait at +* least until until the rcu_read_unlock. +*/ +rcu_read_lock (); cur_devices = fs_info->fs_devices;while (cur_devices) { head = &cur_devices->devices; -list_for_each_entry (dev, head, dev_list) { +list_for_each_entry_rcu (dev, head, dev_list) {if (test_bit (BTRFS_DEV_STATE_MISSING , &dev->dev_state))continue ;if (!dev->name)
@@ -2306,14 +2313,12 @@ static int btrfs_show_devname(struct seq_file *m, struct dentry *root) }if (first_dev) { -rcu_read_lock (); name =rcu_dereference (first_dev->name);seq_escape (m, name->str," \t\n\\" ); -rcu_read_unlock (); }else {WARN_ON (1 ); } -mutex_unlock (&fs_info->fs_devices->device_list_mutex); +rcu_read_unlock ();return 0 ; }
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 5/6 btrfs: remove stale comments about fs_mutex
In reply to David SterbaThe fs_mutex has been killed in 2008, a213501153fd66e2 ("Btrfs: Replace the big fs_mutex with a collection of other locks"), still remembered in some comments.
We don't have any extra needs for locking in the ACL handlers.
--- fs/btrfs/acl.c | 8 -------- 1 file changed, 8 deletions(-)
fs/btrfs/acl.c +0 -8
@@ -65,9 +65,6 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)return acl; } -/* -* Needs to be called with fs_mutex held -*/ static int __btrfs_set_acl (struct btrfs_trans_handle *trans,struct inode *inode,struct posix_acl *acl,int type) {
@@ -127,11 +124,6 @@ int btrfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)return ret; } -/* -* btrfs_init_acl is already generally called under fs_mutex, so the locking -* stuff has been fixed to work with that. If the locking stuff changes, we -* need to re-evaluate the acl locking stuff. -*/ int btrfs_init_acl (struct btrfs_trans_handle *trans,struct inode *inode,struct inode *dir) {
Signature · 1 line
2.16.2
David Sterba
maintainerPATCH 6/6 btrfs: split dev-replace locking helpers for read and write
In reply to David SterbaThe current calls are unclear in what way btrfs_dev_replace_lock takes the locks, so drop the argument, split the helpers and use similar naming as for read and write locks.
--- fs/btrfs/dev-replace.c | 98 +++++++++++++++++++++++++------------------------- fs/btrfs/dev-replace.h | 6 ++-- fs/btrfs/reada.c | 10 +++--- fs/btrfs/scrub.c | 14 ++++---- fs/btrfs/volumes.c | 18 +++++----- 5 files changed, 74 insertions(+), 72 deletions(-)
fs/btrfs/dev-replace.c +49 -49
@@ -200,13 +200,13 @@ int btrfs_run_dev_replace(struct btrfs_trans_handle *trans,struct btrfs_dev_replace_item *ptr;struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace; -btrfs_dev_replace_lock (dev_replace,0 ); +btrfs_dev_replace_read_lock (dev_replace);if (!dev_replace->is_valid || !dev_replace->item_needs_writeback) { -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace);return 0 ; } -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace); key.objectid =0 ; key.type =BTRFS_DEV_REPLACE_KEY ;
@@ -264,7 +264,7 @@ int btrfs_run_dev_replace(struct btrfs_trans_handle *trans, ptr =btrfs_item_ptr (eb, path->slots[0 ], struct btrfs_dev_replace_item); -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace);if (dev_replace->srcdev)btrfs_set_dev_replace_src_devid (eb, ptr, dev_replace->srcdev->devid);
@@ -287,7 +287,7 @@ int btrfs_run_dev_replace(struct btrfs_trans_handle *trans,btrfs_set_dev_replace_cursor_right (eb, ptr, dev_replace->cursor_right); dev_replace->item_needs_writeback =0 ; -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);btrfs_mark_buffer_dirty (eb);
@@ -352,7 +352,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,return PTR_ERR (trans); } -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace);switch (dev_replace->replace_state) {case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED :case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED :
@@ -390,7 +390,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info, dev_replace->item_needs_writeback =1 ;atomic64_set (&dev_replace->num_write_errors,0 );atomic64_set (&dev_replace->num_uncorrectable_read_errors,0 ); -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace); ret =btrfs_sysfs_add_device_link (tgt_device->fs_devices, tgt_device);if (ret)
@@ -402,7 +402,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info, trans =btrfs_start_transaction (root,0 );if (IS_ERR (trans)) { ret =PTR_ERR (trans); -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace); goto leave; }
@@ -426,7 +426,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info, leave: dev_replace->srcdev =NULL ; dev_replace->tgtdev =NULL ; -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);btrfs_destroy_dev_replace_tgtdev (fs_info, tgt_device);return ret; }
@@ -493,18 +493,18 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,/* don't allow cancel or unmount to disturb the finishing procedure */ mutex_lock (&dev_replace->lock_finishing_cancel_unmount); -btrfs_dev_replace_lock (dev_replace,0 ); +btrfs_dev_replace_read_lock (dev_replace);/* was the operation canceled, or is it finished? */ if (dev_replace->replace_state !=BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED ) { -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace);mutex_unlock (&dev_replace->lock_finishing_cancel_unmount);return 0 ; } tgt_device = dev_replace->tgtdev; src_device = dev_replace->srcdev; -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace);/* * flush all outstanding I/O and inode extent mappings before the
@@ -529,7 +529,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,/* keep away write_all_supers() during the finishing procedure */ mutex_lock (&fs_info->fs_devices->device_list_mutex);mutex_lock (&fs_info->chunk_mutex); -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace); dev_replace->replace_state = scrub_ret ?BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED :BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED ;
@@ -549,7 +549,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,btrfs_dev_name (src_device), src_device->devid, rcu_str_deref(tgt_device->name), scrub_ret); -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);mutex_unlock (&fs_info->chunk_mutex);mutex_unlock (&fs_info->fs_devices->device_list_mutex);mutex_unlock (&uuid_mutex);
@@ -586,7 +586,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,list_add (&tgt_device->dev_alloc_list, &fs_info->fs_devices->alloc_list); fs_info->fs_devices->rw_devices++; -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);btrfs_rm_dev_replace_blocked (fs_info);
@@ -679,7 +679,7 @@ void btrfs_dev_replace_status(struct btrfs_fs_info *fs_info, {struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace; -btrfs_dev_replace_lock (dev_replace,0 ); +btrfs_dev_replace_read_lock (dev_replace);/* even if !dev_replace_is_valid, the values are good enough for * the replace_status ioctl */ args->result =BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR ;
@@ -691,7 +691,7 @@ void btrfs_dev_replace_status(struct btrfs_fs_info *fs_info, args->status.num_uncorrectable_read_errors =atomic64_read (&dev_replace->num_uncorrectable_read_errors); args->status.progress_1000 =btrfs_dev_replace_progress (fs_info); -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace); } int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info,
@@ -714,13 +714,13 @@ static u64 __btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info)return -EROFS ;mutex_lock (&dev_replace->lock_finishing_cancel_unmount); -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace);switch (dev_replace->replace_state) {case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED :case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED :case BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED : result =BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED ; -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace); goto leave;case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED :case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED :
@@ -733,7 +733,7 @@ static u64 __btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info) dev_replace->replace_state =BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED ; dev_replace->time_stopped =get_seconds (); dev_replace->item_needs_writeback =1 ; -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);btrfs_scrub_cancel (fs_info); trans =btrfs_start_transaction (root,0 );
@@ -756,7 +756,7 @@ void btrfs_dev_replace_suspend_for_unmount(struct btrfs_fs_info *fs_info)struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;mutex_lock (&dev_replace->lock_finishing_cancel_unmount); -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace);switch (dev_replace->replace_state) {case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED :case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED :
@@ -772,7 +772,7 @@ void btrfs_dev_replace_suspend_for_unmount(struct btrfs_fs_info *fs_info)break ; } -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);mutex_unlock (&dev_replace->lock_finishing_cancel_unmount); }
@@ -782,12 +782,12 @@ int btrfs_resume_dev_replace_async(struct btrfs_fs_info *fs_info)struct task_struct *task;struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace; -btrfs_dev_replace_lock (dev_replace,1 ); +btrfs_dev_replace_write_lock (dev_replace);switch (dev_replace->replace_state) {case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED :case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED :case BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED : -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);return 0 ;case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED :break ;
@@ -801,10 +801,10 @@ int btrfs_resume_dev_replace_async(struct btrfs_fs_info *fs_info) "cannot continue dev_replace, tgtdev is missing");btrfs_info (fs_info,"you may cancel the operation after 'mount -o degraded'" ); -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);return 0 ; } -btrfs_dev_replace_unlock (dev_replace,1 ); +btrfs_dev_replace_write_unlock (dev_replace);WARN_ON (test_and_set_bit (BTRFS_FS_EXCL_OP , &fs_info->flags)); task =kthread_run (btrfs_dev_replace_kthread, fs_info,"btrfs-devrepl" );
@@ -873,37 +873,37 @@ int btrfs_dev_replace_is_ongoing(struct btrfs_dev_replace *dev_replace)return 1 ; } -void btrfs_dev_replace_lock (struct btrfs_dev_replace *dev_replace,int rw) +void btrfs_dev_replace_read_lock (struct btrfs_dev_replace *dev_replace) { -if (rw ==1 ) { -/* write */ -again: -wait_event (dev_replace->read_lock_wq, -atomic_read (&dev_replace->blocking_readers) ==0 ); -write_lock (&dev_replace->lock); -if (atomic_read (&dev_replace->blocking_readers)) { -write_unlock (&dev_replace->lock); - goto again; - } - }else { -read_lock (&dev_replace->lock); -atomic_inc (&dev_replace->read_locks); - } +read_lock (&dev_replace->lock); +atomic_inc (&dev_replace->read_locks); } -void btrfs_dev_replace_unlock (struct btrfs_dev_replace *dev_replace,int rw) +void btrfs_dev_replace_read_unlock (struct btrfs_dev_replace *dev_replace) { -if (rw ==1 ) { -/* write */ -ASSERT (atomic_read (&dev_replace->blocking_readers) ==0 ); +ASSERT (atomic_read (&dev_replace->read_locks) >0 ); +atomic_dec (&dev_replace->read_locks); +read_unlock (&dev_replace->lock); +} + +void btrfs_dev_replace_write_lock (struct btrfs_dev_replace *dev_replace) +{ +again: +wait_event (dev_replace->read_lock_wq, +atomic_read (&dev_replace->blocking_readers) ==0 ); +write_lock (&dev_replace->lock); +if (atomic_read (&dev_replace->blocking_readers)) {write_unlock (&dev_replace->lock); - }else { -ASSERT (atomic_read (&dev_replace->read_locks) >0 ); -atomic_dec (&dev_replace->read_locks); -read_unlock (&dev_replace->lock); + goto again; } } +void btrfs_dev_replace_write_unlock (struct btrfs_dev_replace *dev_replace) +{ +ASSERT (atomic_read (&dev_replace->blocking_readers) ==0 ); +write_unlock (&dev_replace->lock); +} +/* inc blocking cnt and release read lock */ void btrfs_dev_replace_set_lock_blocking (struct btrfs_dev_replace *dev_replace)
fs/btrfs/dev-replace.h +4 -2
@@ -37,8 +37,10 @@ int btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info,void btrfs_dev_replace_suspend_for_unmount (struct btrfs_fs_info *fs_info);int btrfs_resume_dev_replace_async (struct btrfs_fs_info *fs_info);int btrfs_dev_replace_is_ongoing (struct btrfs_dev_replace *dev_replace); -void btrfs_dev_replace_lock (struct btrfs_dev_replace *dev_replace,int rw); -void btrfs_dev_replace_unlock (struct btrfs_dev_replace *dev_replace,int rw); +void btrfs_dev_replace_read_lock (struct btrfs_dev_replace *dev_replace); +void btrfs_dev_replace_read_unlock (struct btrfs_dev_replace *dev_replace); +void btrfs_dev_replace_write_lock (struct btrfs_dev_replace *dev_replace); +void btrfs_dev_replace_write_unlock (struct btrfs_dev_replace *dev_replace);void btrfs_dev_replace_set_lock_blocking (struct btrfs_dev_replace *dev_replace);void btrfs_dev_replace_clear_lock_blocking (struct btrfs_dev_replace *dev_replace);
fs/btrfs/reada.c +5 -5
@@ -395,20 +395,20 @@ static struct reada_extent *reada_find_extent(struct btrfs_fs_info *fs_info, goto error;/* insert extent in reada_tree + all per-device trees, all or nothing */ -btrfs_dev_replace_lock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_lock (&fs_info->dev_replace);spin_lock (&fs_info->reada_lock); ret =radix_tree_insert (&fs_info->reada_tree, index, re);if (ret == -EEXIST ) { re_exist =radix_tree_lookup (&fs_info->reada_tree, index); re_exist->refcnt++;spin_unlock (&fs_info->reada_lock); -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace);radix_tree_preload_end (); goto error; }if (ret) {spin_unlock (&fs_info->reada_lock); -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace);radix_tree_preload_end (); goto error; }
@@ -451,13 +451,13 @@ static struct reada_extent *reada_find_extent(struct btrfs_fs_info *fs_info, }radix_tree_delete (&fs_info->reada_tree, index);spin_unlock (&fs_info->reada_lock); -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace); goto error; } have_zone =1 ; }spin_unlock (&fs_info->reada_lock); -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace);if (!have_zone) goto error;
fs/btrfs/scrub.c +7 -7
@@ -3885,11 +3885,11 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,break ; } -btrfs_dev_replace_lock (&fs_info->dev_replace,1 ); +btrfs_dev_replace_write_lock (&fs_info->dev_replace); dev_replace->cursor_right = found_key.offset + length; dev_replace->cursor_left = found_key.offset; dev_replace->item_needs_writeback =1 ; -btrfs_dev_replace_unlock (&fs_info->dev_replace,1 ); +btrfs_dev_replace_write_unlock (&fs_info->dev_replace); ret =scrub_chunk (sctx, scrub_dev, chunk_offset, length, found_key.offset, cache, is_dev_replace);
@@ -3925,10 +3925,10 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,scrub_pause_off (fs_info); -btrfs_dev_replace_lock (&fs_info->dev_replace,1 ); +btrfs_dev_replace_write_lock (&fs_info->dev_replace); dev_replace->cursor_left = dev_replace->cursor_right; dev_replace->item_needs_writeback =1 ; -btrfs_dev_replace_unlock (&fs_info->dev_replace,1 ); +btrfs_dev_replace_write_unlock (&fs_info->dev_replace);if (ro_set)btrfs_dec_block_group_ro (cache);
@@ -4144,16 +4144,16 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,return -EIO ; } -btrfs_dev_replace_lock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_lock (&fs_info->dev_replace);if (dev->scrub_ctx || (!is_dev_replace &&btrfs_dev_replace_is_ongoing (&fs_info->dev_replace))) { -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace);mutex_unlock (&fs_info->scrub_lock);mutex_unlock (&fs_info->fs_devices->device_list_mutex);return -EINPROGRESS ; } -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace); ret =scrub_workers_get (fs_info, is_dev_replace);if (ret) {
fs/btrfs/volumes.c +9 -9
@@ -1916,12 +1916,12 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,mutex_lock (&uuid_mutex); num_devices = fs_info->fs_devices->num_devices; -btrfs_dev_replace_lock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_lock (&fs_info->dev_replace);if (btrfs_dev_replace_is_ongoing (&fs_info->dev_replace)) {WARN_ON (num_devices <1 ); num_devices--; } -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace); ret =btrfs_check_raid_min_devices (fs_info, num_devices -1 );if (ret)
@@ -3892,12 +3892,12 @@ int btrfs_balance(struct btrfs_balance_control *bctl, } num_devices = fs_info->fs_devices->num_devices; -btrfs_dev_replace_lock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_lock (&fs_info->dev_replace);if (btrfs_dev_replace_is_ongoing (&fs_info->dev_replace)) {BUG_ON (num_devices <1 ); num_devices--; } -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace); allowed =BTRFS_AVAIL_ALLOC_BIT_SINGLE |BTRFS_BLOCK_GROUP_DUP ;if (num_devices >1 ) allowed |= (BTRFS_BLOCK_GROUP_RAID0 |BTRFS_BLOCK_GROUP_RAID1 );
@@ -5209,11 +5209,11 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len) ret =1 ;free_extent_map (em); -btrfs_dev_replace_lock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_lock (&fs_info->dev_replace);if (btrfs_dev_replace_is_ongoing (&fs_info->dev_replace) && fs_info->dev_replace.tgtdev) ret++; -btrfs_dev_replace_unlock (&fs_info->dev_replace,0 ); +btrfs_dev_replace_read_unlock (&fs_info->dev_replace);return ret; }
@@ -5779,10 +5779,10 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,if (!bbio_ret) goto out; -btrfs_dev_replace_lock (dev_replace,0 ); +btrfs_dev_replace_read_lock (dev_replace); dev_replace_is_ongoing =btrfs_dev_replace_is_ongoing (dev_replace);if (!dev_replace_is_ongoing) -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace);else btrfs_dev_replace_set_lock_blocking (dev_replace);
@@ -5984,7 +5984,7 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, out:if (dev_replace_is_ongoing) {btrfs_dev_replace_clear_lock_blocking (dev_replace); -btrfs_dev_replace_unlock (dev_replace,0 ); +btrfs_dev_replace_read_unlock (dev_replace); }free_extent_map (em);return ret;
Signature · 1 line
2.16.2
Re: [PATCH 4/6] btrfs: use RCU in btrfs_show_devname for device list traversal
In reply to David SterbaQuoted 11 lines · David Sterba
The
show_devnamecallback is used to print device name in/proc/self/mounts, we need to traverse the device list consistently and read the name that's copied to a seq buffer so we don't need further locking.If the first device is being deleted at the same time, the RCU will allow us to read the device name, though it will become stale right after the RCU protection ends. This is unavoidable and the user can expect that the device will disappear from the filesystem's list at some point.
Quoted 5 lines
The
device_list_mutexwas pretty heavy as it is used eg. for writing superblock and a few other IO related contexts. This can stall any application that reads the proc file for no reason.Signed-off-by David Sterba <[email protected]>