* [PATCH v4 2/2] btrfs: get device pointer from btrfs_scan_one_device
2018-07-12 6:23 [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Gu Jinxiang
@ 2018-07-12 6:23 ` Gu Jinxiang
2018-07-12 6:31 ` Nikolay Borisov
2018-07-12 6:31 ` [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Nikolay Borisov
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Gu Jinxiang @ 2018-07-12 6:23 UTC (permalink / raw)
To: linux-btrfs; +Cc: anand.jain, nborisov
Instead of pointer to btrfs_fs_devices as an arg in
btrfs_scan_one_device, better to make it as a return value.
And since btrfs_fs_devices can be get by btrfs_device,
better to return btrfs_device than fs_btrfs_devices.
Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
---
Changelog:
v4: as suggested by Anand, change return value of
btrfs_scan_one_device from btrfs_fs_devices to btrfs_device.
v3: as comment by robot, use PTR_ERR_OR_ZERO, and rebase to misc-next.
v2: as comment by Nikolay, use ERR_CAST instead of cast type manually.
fs/btrfs/super.c | 37 ++++++++++++++++++++++++-------------
fs/btrfs/volumes.c | 17 ++++++-----------
fs/btrfs/volumes.h | 4 ++--
3 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 44f58bdb5fa6..e73d547eacff 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -888,7 +888,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
{
substring_t args[MAX_OPT_ARGS];
char *device_name, *opts, *orig, *p;
- struct btrfs_fs_devices *fs_devices = NULL;
+ struct btrfs_device *device = NULL;
int error = 0;
if (!options)
@@ -916,11 +916,13 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
error = -ENOMEM;
goto out;
}
- error = btrfs_scan_one_device(device_name,
- flags, holder, &fs_devices);
+ device = btrfs_scan_one_device(device_name,
+ flags, holder);
kfree(device_name);
- if (error)
+ if (IS_ERR(device)) {
+ error = PTR_ERR(device);
goto out;
+ }
}
}
@@ -1516,6 +1518,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
{
struct block_device *bdev = NULL;
struct super_block *s;
+ struct btrfs_device *device = NULL;
struct btrfs_fs_devices *fs_devices = NULL;
struct btrfs_fs_info *fs_info = NULL;
struct security_mnt_opts new_sec_opts;
@@ -1537,9 +1540,13 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
return ERR_PTR(error);
}
- error = btrfs_scan_one_device(device_name, mode, fs_type, &fs_devices);
- if (error)
+ device = btrfs_scan_one_device(device_name, mode, fs_type);
+ if (IS_ERR(device)) {
+ error = PTR_ERR(device);
goto error_sec_opts;
+ }
+
+ fs_devices = device->fs_devices;
/*
* Setup a dummy root and fs_info for test/set super. This is because
@@ -2220,7 +2227,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct btrfs_ioctl_vol_args *vol;
- struct btrfs_fs_devices *fs_devices;
+ struct btrfs_device *device = NULL;
int ret = -ENOTTY;
if (!capable(CAP_SYS_ADMIN))
@@ -2232,15 +2239,19 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
switch (cmd) {
case BTRFS_IOC_SCAN_DEV:
- ret = btrfs_scan_one_device(vol->name, FMODE_READ,
- &btrfs_root_fs_type, &fs_devices);
+ device = btrfs_scan_one_device(vol->name, FMODE_READ,
+ &btrfs_root_fs_type);
+ ret = PTR_ERR_OR_ZERO(device);
break;
case BTRFS_IOC_DEVICES_READY:
- ret = btrfs_scan_one_device(vol->name, FMODE_READ,
- &btrfs_root_fs_type, &fs_devices);
- if (ret)
+ device = btrfs_scan_one_device(vol->name, FMODE_READ,
+ &btrfs_root_fs_type);
+ if (IS_ERR(device)) {
+ ret = PTR_ERR(device);
break;
- ret = !(fs_devices->num_devices == fs_devices->total_devices);
+ }
+ ret = !(device->fs_devices->num_devices ==
+ device->fs_devices->total_devices);
break;
case BTRFS_IOC_GET_SUPPORTED_FEATURES:
ret = btrfs_ioctl_get_supported_features((void __user*)arg);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 52abada5c789..3bc479e8cc22 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1212,14 +1212,13 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
* and we are not allowed to call set_blocksize during the scan. The superblock
* is read via pagecache
*/
-int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
- struct btrfs_fs_devices **fs_devices_ret)
+struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags,
+ void *holder)
{
struct btrfs_super_block *disk_super;
- struct btrfs_device *device;
+ struct btrfs_device *ret = NULL;
struct block_device *bdev;
struct page *page;
- int ret = 0;
u64 bytenr;
/*
@@ -1233,19 +1232,15 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
bdev = blkdev_get_by_path(path, flags, holder);
if (IS_ERR(bdev))
- return PTR_ERR(bdev);
+ return ERR_CAST(bdev);
if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
- ret = -EINVAL;
+ ret = ERR_PTR(-EINVAL);
goto error_bdev_put;
}
mutex_lock(&uuid_mutex);
- device = device_list_add(path, disk_super);
- if (IS_ERR(device))
- ret = PTR_ERR(device);
- else
- *fs_devices_ret = device->fs_devices;
+ ret = device_list_add(path, disk_super);
mutex_unlock(&uuid_mutex);
btrfs_release_disk_super(page);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 9665b84b1026..06d8bb7dd557 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -403,8 +403,8 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
int mirror_num, int async_submit);
int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
fmode_t flags, void *holder);
-int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
- struct btrfs_fs_devices **fs_devices_ret);
+struct btrfs_device *btrfs_scan_one_device(const char *path,
+ fmode_t flags, void *holder);
int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
void btrfs_assign_next_active_device(struct btrfs_fs_info *fs_info,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4 2/2] btrfs: get device pointer from btrfs_scan_one_device
2018-07-12 6:23 ` [PATCH v4 2/2] btrfs: get device pointer from btrfs_scan_one_device Gu Jinxiang
@ 2018-07-12 6:31 ` Nikolay Borisov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2018-07-12 6:31 UTC (permalink / raw)
To: Gu Jinxiang, linux-btrfs; +Cc: anand.jain
On 12.07.2018 09:23, Gu Jinxiang wrote:
> Instead of pointer to btrfs_fs_devices as an arg in
> btrfs_scan_one_device, better to make it as a return value.
>
> And since btrfs_fs_devices can be get by btrfs_device,
> better to return btrfs_device than fs_btrfs_devices.
>
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Changelog:
> v4: as suggested by Anand, change return value of
> btrfs_scan_one_device from btrfs_fs_devices to btrfs_device.
> v3: as comment by robot, use PTR_ERR_OR_ZERO, and rebase to misc-next.
> v2: as comment by Nikolay, use ERR_CAST instead of cast type manually.
>
> fs/btrfs/super.c | 37 ++++++++++++++++++++++++-------------
> fs/btrfs/volumes.c | 17 ++++++-----------
> fs/btrfs/volumes.h | 4 ++--
> 3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 44f58bdb5fa6..e73d547eacff 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -888,7 +888,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
> {
> substring_t args[MAX_OPT_ARGS];
> char *device_name, *opts, *orig, *p;
> - struct btrfs_fs_devices *fs_devices = NULL;
> + struct btrfs_device *device = NULL;
> int error = 0;
>
> if (!options)
> @@ -916,11 +916,13 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
> error = -ENOMEM;
> goto out;
> }
> - error = btrfs_scan_one_device(device_name,
> - flags, holder, &fs_devices);
> + device = btrfs_scan_one_device(device_name,
> + flags, holder);
> kfree(device_name);
> - if (error)
> + if (IS_ERR(device)) {
> + error = PTR_ERR(device);
> goto out;
> + }
> }
> }
>
> @@ -1516,6 +1518,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> {
> struct block_device *bdev = NULL;
> struct super_block *s;
> + struct btrfs_device *device = NULL;
> struct btrfs_fs_devices *fs_devices = NULL;
> struct btrfs_fs_info *fs_info = NULL;
> struct security_mnt_opts new_sec_opts;
> @@ -1537,9 +1540,13 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> return ERR_PTR(error);
> }
>
> - error = btrfs_scan_one_device(device_name, mode, fs_type, &fs_devices);
> - if (error)
> + device = btrfs_scan_one_device(device_name, mode, fs_type);
> + if (IS_ERR(device)) {
> + error = PTR_ERR(device);
> goto error_sec_opts;
> + }
> +
> + fs_devices = device->fs_devices;
>
> /*
> * Setup a dummy root and fs_info for test/set super. This is because
> @@ -2220,7 +2227,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> struct btrfs_ioctl_vol_args *vol;
> - struct btrfs_fs_devices *fs_devices;
> + struct btrfs_device *device = NULL;
> int ret = -ENOTTY;
>
> if (!capable(CAP_SYS_ADMIN))
> @@ -2232,15 +2239,19 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>
> switch (cmd) {
> case BTRFS_IOC_SCAN_DEV:
> - ret = btrfs_scan_one_device(vol->name, FMODE_READ,
> - &btrfs_root_fs_type, &fs_devices);
> + device = btrfs_scan_one_device(vol->name, FMODE_READ,
> + &btrfs_root_fs_type);
> + ret = PTR_ERR_OR_ZERO(device);
> break;
> case BTRFS_IOC_DEVICES_READY:
> - ret = btrfs_scan_one_device(vol->name, FMODE_READ,
> - &btrfs_root_fs_type, &fs_devices);
> - if (ret)
> + device = btrfs_scan_one_device(vol->name, FMODE_READ,
> + &btrfs_root_fs_type);
> + if (IS_ERR(device)) {
> + ret = PTR_ERR(device);
> break;
> - ret = !(fs_devices->num_devices == fs_devices->total_devices);
> + }
> + ret = !(device->fs_devices->num_devices ==
> + device->fs_devices->total_devices);
> break;
> case BTRFS_IOC_GET_SUPPORTED_FEATURES:
> ret = btrfs_ioctl_get_supported_features((void __user*)arg);
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 52abada5c789..3bc479e8cc22 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1212,14 +1212,13 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
> * and we are not allowed to call set_blocksize during the scan. The superblock
> * is read via pagecache
> */
> -int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
> - struct btrfs_fs_devices **fs_devices_ret)
> +struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags,
> + void *holder)
> {
> struct btrfs_super_block *disk_super;
> - struct btrfs_device *device;
> + struct btrfs_device *ret = NULL;
> struct block_device *bdev;
> struct page *page;
> - int ret = 0;
> u64 bytenr;
>
> /*
> @@ -1233,19 +1232,15 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
>
> bdev = blkdev_get_by_path(path, flags, holder);
> if (IS_ERR(bdev))
> - return PTR_ERR(bdev);
> + return ERR_CAST(bdev);
>
> if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
> - ret = -EINVAL;
> + ret = ERR_PTR(-EINVAL);
> goto error_bdev_put;
> }
>
> mutex_lock(&uuid_mutex);
> - device = device_list_add(path, disk_super);
> - if (IS_ERR(device))
> - ret = PTR_ERR(device);
> - else
> - *fs_devices_ret = device->fs_devices;
> + ret = device_list_add(path, disk_super);
> mutex_unlock(&uuid_mutex);
>
> btrfs_release_disk_super(page);
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 9665b84b1026..06d8bb7dd557 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -403,8 +403,8 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
> int mirror_num, int async_submit);
> int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
> fmode_t flags, void *holder);
> -int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
> - struct btrfs_fs_devices **fs_devices_ret);
> +struct btrfs_device *btrfs_scan_one_device(const char *path,
> + fmode_t flags, void *holder);
> int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
> void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
> void btrfs_assign_next_active_device(struct btrfs_fs_info *fs_info,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] btrfs: make fs_devices to be a local variable
2018-07-12 6:23 [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Gu Jinxiang
2018-07-12 6:23 ` [PATCH v4 2/2] btrfs: get device pointer from btrfs_scan_one_device Gu Jinxiang
@ 2018-07-12 6:31 ` Nikolay Borisov
2018-07-13 15:16 ` David Sterba
2018-07-16 5:45 ` Anand Jain
3 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2018-07-12 6:31 UTC (permalink / raw)
To: Gu Jinxiang, linux-btrfs; +Cc: anand.jain
On 12.07.2018 09:23, Gu Jinxiang wrote:
> fs_devices is always passed to btrfs_scan_one_device which
> overrides it. And in the call stack below fs_devices is passed to
> btrfs_scan_one_device from btrfs_mount_root.
> And in btrfs_mount_root the output fs_devices of this call stack
> is not used.
> btrfs_mount_root
> -> btrfs_parse_early_options
> ->btrfs_scan_one_device
> So, there is no necessary to pass fs_devices from btrfs_mount_root,
> use a local variable in btrfs_parse_early_options is enough.
>
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
> Reviewed-by: Anand Jain <Anand.Jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Changelog:
> v4: changed a line warp, and adjusted the order of two rows.
> v3: rebase to misc-next.
> v2: deal with Nikolay's comment, make changelog more clair.
>
> fs/btrfs/super.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 39d8e39b2fe1..44f58bdb5fa6 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -884,10 +884,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> * only when we need to allocate a new super block.
> */
> static int btrfs_parse_early_options(const char *options, fmode_t flags,
> - void *holder, struct btrfs_fs_devices **fs_devices)
> + void *holder)
> {
> substring_t args[MAX_OPT_ARGS];
> char *device_name, *opts, *orig, *p;
> + struct btrfs_fs_devices *fs_devices = NULL;
> int error = 0;
>
> if (!options)
> @@ -916,7 +917,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
> goto out;
> }
> error = btrfs_scan_one_device(device_name,
> - flags, holder, fs_devices);
> + flags, holder, &fs_devices);
> kfree(device_name);
> if (error)
> goto out;
> @@ -1524,8 +1525,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> if (!(flags & SB_RDONLY))
> mode |= FMODE_WRITE;
>
> - error = btrfs_parse_early_options(data, mode, fs_type,
> - &fs_devices);
> + error = btrfs_parse_early_options(data, mode, fs_type);
> if (error) {
> return ERR_PTR(error);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4 1/2] btrfs: make fs_devices to be a local variable
2018-07-12 6:23 [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Gu Jinxiang
2018-07-12 6:23 ` [PATCH v4 2/2] btrfs: get device pointer from btrfs_scan_one_device Gu Jinxiang
2018-07-12 6:31 ` [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Nikolay Borisov
@ 2018-07-13 15:16 ` David Sterba
2018-07-16 5:45 ` Anand Jain
3 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2018-07-13 15:16 UTC (permalink / raw)
To: Gu Jinxiang; +Cc: linux-btrfs, anand.jain, nborisov
On Thu, Jul 12, 2018 at 02:23:15PM +0800, Gu Jinxiang wrote:
> fs_devices is always passed to btrfs_scan_one_device which
> overrides it. And in the call stack below fs_devices is passed to
> btrfs_scan_one_device from btrfs_mount_root.
> And in btrfs_mount_root the output fs_devices of this call stack
> is not used.
> btrfs_mount_root
> -> btrfs_parse_early_options
> ->btrfs_scan_one_device
> So, there is no necessary to pass fs_devices from btrfs_mount_root,
> use a local variable in btrfs_parse_early_options is enough.
>
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
> Reviewed-by: Anand Jain <Anand.Jain@oracle.com>
Please send cover letter for patch series (ie. > 1 patch).
I've applied both on top of recent patches that update some code around
device scanning and there were conflicts with patch 2, but simple to fix.
Please check the result in misc-next once it's pushed, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] btrfs: make fs_devices to be a local variable
2018-07-12 6:23 [PATCH v4 1/2] btrfs: make fs_devices to be a local variable Gu Jinxiang
` (2 preceding siblings ...)
2018-07-13 15:16 ` David Sterba
@ 2018-07-16 5:45 ` Anand Jain
3 siblings, 0 replies; 6+ messages in thread
From: Anand Jain @ 2018-07-16 5:45 UTC (permalink / raw)
To: Gu Jinxiang, linux-btrfs; +Cc: nborisov
On 07/12/2018 02:23 PM, Gu Jinxiang wrote:
> fs_devices is always passed to btrfs_scan_one_device which
> overrides it. And in the call stack below fs_devices is passed to
> btrfs_scan_one_device from btrfs_mount_root.
> And in btrfs_mount_root the output fs_devices of this call stack
> is not used.
> btrfs_mount_root
> -> btrfs_parse_early_options
> ->btrfs_scan_one_device
> So, there is no necessary to pass fs_devices from btrfs_mount_root,
> use a local variable in btrfs_parse_early_options is enough.
>
> Signed-off-by: Gu Jinxiang <gujx@cn.fujitsu.com>
> Reviewed-by: Anand Jain <Anand.Jain@oracle.com>
> ---
>
> Changelog:
> v4: changed a line warp, and adjusted the order of two rows.
> v3: rebase to misc-next.
> v2: deal with Nikolay's comment, make changelog more clair.
>
> fs/btrfs/super.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 39d8e39b2fe1..44f58bdb5fa6 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -884,10 +884,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> * only when we need to allocate a new super block.
> */
> static int btrfs_parse_early_options(const char *options, fmode_t flags,
> - void *holder, struct btrfs_fs_devices **fs_devices)
> + void *holder)
> {
> substring_t args[MAX_OPT_ARGS];
> char *device_name, *opts, *orig, *p;
> + struct btrfs_fs_devices *fs_devices = NULL;
> int error = 0;
>
> if (!options)
> @@ -916,7 +917,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
> goto out;
> }
> error = btrfs_scan_one_device(device_name,
> - flags, holder, fs_devices);
> + flags, holder, &fs_devices);
1.
Extend line until 8-char, and remain to start below the (.
- error = btrfs_scan_one_device(device_name,
- flags, holder, fs_devices);
+ error = btrfs_scan_one_device(device_name, flags,
+ holder, &fs_devices);
2.
Fixed the conflict on top of the recent uuid_mutex changes from David.
I see this patch isn't in the misc-next yet, applied to my local WS,
pending pull request.
Thanks, Anand
> kfree(device_name);
> if (error)
> goto out;
> @@ -1524,8 +1525,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
> if (!(flags & SB_RDONLY))
> mode |= FMODE_WRITE;
>
> - error = btrfs_parse_early_options(data, mode, fs_type,
> - &fs_devices);
> + error = btrfs_parse_early_options(data, mode, fs_type);
> if (error) {
> return ERR_PTR(error);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread