| Message ID | 8430e9c00d58748f1cb574b686396c8349ac7311.1509471604.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 31.10.2017 19:44, David Sterba wrote: > We take the fs_devices::device_list_mutex mutex in write_all_supers > which will prevent any add/del changes to the device list. Therefore we > don't need to use the RCU variant list_for_each_entry_rcu in any of the > called functions. > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/disk-io.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index efce9a2fa9be..042cf46e4cd0 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3396,9 +3396,10 @@ static int barrier_all_devices(struct btrfs_fs_info *info) > int errors_wait = 0; > blk_status_t ret; > > + WARN_ON(!mutex_is_locked(&info->fs_devices->device_list_mutex)); Don't we want lockdep_assert_held ? I'm assuming enough testing with lockdep on is performed so it will be caught in time if this invariant is broken. > /* send down all the barriers */ > head = &info->fs_devices->devices; > - list_for_each_entry_rcu(dev, head, dev_list) { > + list_for_each_entry(dev, head, dev_list) { > if (dev->missing) > continue; > if (!dev->bdev) > @@ -3411,7 +3412,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info) > } > > /* wait for all the barriers */ > - list_for_each_entry_rcu(dev, head, dev_list) { > + list_for_each_entry(dev, head, dev_list) { > if (dev->missing) > continue; > if (!dev->bdev) { > @@ -3510,7 +3511,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors) > } > } > > - list_for_each_entry_rcu(dev, head, dev_list) { > + list_for_each_entry(dev, head, dev_list) { > if (!dev->bdev) { > total_errors++; > continue; > @@ -3551,7 +3552,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors) > } > > total_errors = 0; > - list_for_each_entry_rcu(dev, head, dev_list) { > + list_for_each_entry(dev, head, dev_list) { > if (!dev->bdev) > continue; > if (!dev->in_fs_metadata || !dev->writeable) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 02, 2017 at 11:49:15AM +0200, Nikolay Borisov wrote: > > > On 31.10.2017 19:44, David Sterba wrote: > > We take the fs_devices::device_list_mutex mutex in write_all_supers > > which will prevent any add/del changes to the device list. Therefore we > > don't need to use the RCU variant list_for_each_entry_rcu in any of the > > called functions. > > > > Signed-off-by: David Sterba <dsterba@suse.com> > > --- > > fs/btrfs/disk-io.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > > index efce9a2fa9be..042cf46e4cd0 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -3396,9 +3396,10 @@ static int barrier_all_devices(struct btrfs_fs_info *info) > > int errors_wait = 0; > > blk_status_t ret; > > > > + WARN_ON(!mutex_is_locked(&info->fs_devices->device_list_mutex)); > > Don't we want lockdep_assert_held ? I'm assuming enough testing with > lockdep on is performed so it will be caught in time if this invariant > is broken. Yes, lockdep_assert_held should enough, this is a development-time warning. Will be fixed. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index efce9a2fa9be..042cf46e4cd0 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3396,9 +3396,10 @@ static int barrier_all_devices(struct btrfs_fs_info *info) int errors_wait = 0; blk_status_t ret; + WARN_ON(!mutex_is_locked(&info->fs_devices->device_list_mutex)); /* send down all the barriers */ head = &info->fs_devices->devices; - list_for_each_entry_rcu(dev, head, dev_list) { + list_for_each_entry(dev, head, dev_list) { if (dev->missing) continue; if (!dev->bdev) @@ -3411,7 +3412,7 @@ static int barrier_all_devices(struct btrfs_fs_info *info) } /* wait for all the barriers */ - list_for_each_entry_rcu(dev, head, dev_list) { + list_for_each_entry(dev, head, dev_list) { if (dev->missing) continue; if (!dev->bdev) { @@ -3510,7 +3511,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors) } } - list_for_each_entry_rcu(dev, head, dev_list) { + list_for_each_entry(dev, head, dev_list) { if (!dev->bdev) { total_errors++; continue; @@ -3551,7 +3552,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors) } total_errors = 0; - list_for_each_entry_rcu(dev, head, dev_list) { + list_for_each_entry(dev, head, dev_list) { if (!dev->bdev) continue; if (!dev->in_fs_metadata || !dev->writeable)
We take the fs_devices::device_list_mutex mutex in write_all_supers which will prevent any add/del changes to the device list. Therefore we don't need to use the RCU variant list_for_each_entry_rcu in any of the called functions. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/disk-io.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)