* [PATCH 2/3] btrfs: optimize move uuid_mutex closer to the critical section
2017-12-15 7:40 [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() Anand Jain
@ 2017-12-15 7:40 ` Anand Jain
2018-01-05 14:11 ` David Sterba
2017-12-15 7:40 ` [PATCH 3/3] btrfs: misc cleanup btrfs_scan_one_device() Anand Jain
2018-01-05 14:09 ` [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() David Sterba
2 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2017-12-15 7:40 UTC (permalink / raw)
To: linux-btrfs
Move uuid_mutex closer to the exclusion section.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/volumes.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a81574dba124..a7eb9b42cd92 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1187,13 +1187,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
*/
bytenr = btrfs_sb_offset(0);
flags |= FMODE_EXCL;
- mutex_lock(&uuid_mutex);
bdev = blkdev_get_by_path(path, flags, holder);
- if (IS_ERR(bdev)) {
- ret = PTR_ERR(bdev);
- goto error;
- }
+ if (IS_ERR(bdev))
+ return PTR_ERR(bdev);
if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super))
goto error_bdev_put;
@@ -1202,7 +1199,12 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
transid = btrfs_super_generation(disk_super);
total_devices = btrfs_super_num_devices(disk_super);
+ mutex_lock(&uuid_mutex);
ret = device_list_add(path, disk_super, devid, fs_devices_ret);
+ if (ret >= 0 && fs_devices_ret)
+ (*fs_devices_ret)->total_devices = total_devices;
+ mutex_unlock(&uuid_mutex);
+
if (ret > 0) {
if (disk_super->label[0]) {
pr_info("BTRFS: device label %s ", disk_super->label);
@@ -1213,15 +1215,12 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
pr_cont("devid %llu transid %llu %s\n", devid, transid, path);
ret = 0;
}
- if (!ret && fs_devices_ret)
- (*fs_devices_ret)->total_devices = total_devices;
btrfs_release_disk_super(page);
error_bdev_put:
blkdev_put(bdev, flags);
-error:
- mutex_unlock(&uuid_mutex);
+
return ret;
}
--
2.7.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] btrfs: misc cleanup btrfs_scan_one_device()
2017-12-15 7:40 [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() Anand Jain
2017-12-15 7:40 ` [PATCH 2/3] btrfs: optimize move uuid_mutex closer to the critical section Anand Jain
@ 2017-12-15 7:40 ` Anand Jain
2018-01-05 14:14 ` David Sterba
2018-01-05 14:09 ` [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() David Sterba
2 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2017-12-15 7:40 UTC (permalink / raw)
To: linux-btrfs
Assign ret = -EINVAL where actually its required.
Remove { } around single line if else code.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/volumes.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a7eb9b42cd92..ba06a4ccd5a6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1173,7 +1173,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
struct btrfs_super_block *disk_super;
struct block_device *bdev;
struct page *page;
- int ret = -EINVAL;
+ int ret;
u64 devid;
u64 transid;
u64 total_devices;
@@ -1192,8 +1192,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
if (IS_ERR(bdev))
return PTR_ERR(bdev);
- if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super))
+ if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
+ ret = -EINVAL;
goto error_bdev_put;
+ }
devid = btrfs_stack_device_id(&disk_super->dev_item);
transid = btrfs_super_generation(disk_super);
@@ -1206,11 +1208,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
mutex_unlock(&uuid_mutex);
if (ret > 0) {
- if (disk_super->label[0]) {
+ if (disk_super->label[0])
pr_info("BTRFS: device label %s ", disk_super->label);
- } else {
+ else
pr_info("BTRFS: device fsid %pU ", disk_super->fsid);
- }
pr_cont("devid %llu transid %llu %s\n", devid, transid, path);
ret = 0;
--
2.7.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device()
2017-12-15 7:40 [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() Anand Jain
2017-12-15 7:40 ` [PATCH 2/3] btrfs: optimize move uuid_mutex closer to the critical section Anand Jain
2017-12-15 7:40 ` [PATCH 3/3] btrfs: misc cleanup btrfs_scan_one_device() Anand Jain
@ 2018-01-05 14:09 ` David Sterba
2018-01-22 4:11 ` Anand Jain
2 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2018-01-05 14:09 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Fri, Dec 15, 2017 at 03:40:14PM +0800, Anand Jain wrote:
> No functional change. First set the usual case, writeable then check
> for any special config.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/volumes.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 5a4c30451c7f..a81574dba124 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -676,14 +676,12 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>
> device->generation = btrfs_super_generation(disk_super);
>
> + set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
I would not say there's no functional change. This line will
unconditionally set the writeable flag, but this was not the case
before.
Sure it's dropped a few lines below, but this would need some checking
that it's not a problem. btrfs_open_one_device is indirectly called from
mount so it should be safe (we can't use one device twice), but this
needs to be documented.
> if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
> clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> fs_devices->seeding = 1;
> - } else {
> - if (bdev_read_only(bdev))
> - clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> - else
> - set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> + } else if (bdev_read_only(bdev)) {
> + clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
> }
>
> q = bdev_get_queue(bdev);
> --
> 2.7.0
>
> --
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device()
2018-01-05 14:09 ` [PATCH 1/3] btrfs: make code easy to read in btrfs_open_one_device() David Sterba
@ 2018-01-22 4:11 ` Anand Jain
0 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2018-01-22 4:11 UTC (permalink / raw)
To: dsterba, linux-btrfs
On 01/05/2018 10:09 PM, David Sterba wrote:
> On Fri, Dec 15, 2017 at 03:40:14PM +0800, Anand Jain wrote:
>> No functional change. First set the usual case, writeable then check
>> for any special config.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> fs/btrfs/volumes.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 5a4c30451c7f..a81574dba124 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -676,14 +676,12 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>>
>> device->generation = btrfs_super_generation(disk_super);
>>
>> + set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>
> I would not say there's no functional change. This line will
> unconditionally set the writeable flag, but this was not the case
> before.
(Sorry for the delay, got distracted by other patches).
agreed here.
> Sure it's dropped a few lines below, but this would need some checking
> that it's not a problem. btrfs_open_one_device is indirectly called from
> mount so it should be safe (we can't use one device twice), but this
> needs to be documented.
Sure will do.
Thanks, Anand
>> if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
>> clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>> fs_devices->seeding = 1;
>> - } else {
>> - if (bdev_read_only(bdev))
>> - clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>> - else
>> - set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>> + } else if (bdev_read_only(bdev)) {
>> + clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>> }
>>
>> q = bdev_get_queue(bdev);
>> --
>> 2.7.0
>>
>> --
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread