Re: [PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
On Tue, Jul 17, 2018 at 12:54:00PM +0800, Anand Jain wrote:
>
> ::
>
> >>> @@ -1561,12 +1564,15 @@ static struct dentry *btrfs_mount_root(struct
> >>> file_system_type *fs_type,
> >>> goto error_fs_info;
> >>> }
> >>>
> >>> - error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
> >>> - if (error) {
> >>> + device = btrfs_scan_one_device(device_name, mode, fs_type);
> >>> + if (IS_ERR(device)) {
> >>> + error = PTR_ERR(device);
> >>> mutex_unlock(_mutex);
> >>> goto error_fs_info;
> >>> }
> >>>
> >>> + fs_devices = device->fs_devices;
> >>> +
>
> The version at misc-next is missing this assignment and it panics.
Thanks, that was my mismerge. Now added back and pushed.
--
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
Re: [PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
On Tue, Jul 17, 2018 at 01:08:34AM +, Gu, Jinxiang wrote:
> > > - device = device_list_add(path, disk_super, _device_added);
> > > - if (IS_ERR(device)) {
> > > - ret = PTR_ERR(device);
> > > - } else {
> > > - *fs_devices_ret = device->fs_devices;
> > > - if (new_device_added)
> > > - btrfs_free_stale_devices(path, device);
> > > - }
> > > + ret = device_list_add(path, disk_super, _device_added);
> > > + if (new_device_added)
> > > + btrfs_free_stale_devices(path, ret);
> >
> > Why is this skipping the error handling? I've resolved the conflict in a
> > different way in misc-next, have you had a look there?
>
> Since when new_device_added set to be true, the return value of
> device_list_add
> must not a error value.
Yeah, but this is not obvious from the context and looks like "
device_list_add fails but we'll do something anyway", so I'm for keeping
the check.
> So, I think no necessary to add the judge.
--
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
Re: [PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
::
@@ -1561,12 +1564,15 @@ static struct dentry *btrfs_mount_root(struct
file_system_type *fs_type,
goto error_fs_info;
}
- error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
- if (error) {
+ device = btrfs_scan_one_device(device_name, mode, fs_type);
+ if (IS_ERR(device)) {
+ error = PTR_ERR(device);
mutex_unlock(_mutex);
goto error_fs_info;
}
+ fs_devices = device->fs_devices;
+
The version at misc-next is missing this assignment and it panics.
::
Thanks, Anand
--
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
RE: [PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
> -Original Message-
> From: David Sterba [mailto:dste...@suse.cz]
> Sent: Monday, July 16, 2018 8:34 PM
> To: Gu, Jinxiang/顾 金香
> Cc: linux-btrfs@vger.kernel.org; anand.j...@oracle.com
> Subject: Re: [PATCH v5 2/2] btrfs: get device pointer from
> btrfs_scan_one_device
>
> On Mon, Jul 16, 2018 at 05:11:17PM +0800, 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
> > ---
> >
> > Changelog:
> > v5: rebase to misc-next.
> > 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 | 35 ++-
> > fs/btrfs/volumes.c | 22 --
> > fs/btrfs/volumes.h | 4 ++--
> > 3 files changed, 32 insertions(+), 29 deletions(-)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index dcab4a0244e5..413bbe2d6db2 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;
> >
> > lockdep_assert_held(_mutex);
> > @@ -918,11 +918,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, _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;
> > + }
> > }
> > }
> >
> > @@ -1518,6 +1520,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;
> > @@ -1561,12 +1564,15 @@ static struct dentry *btrfs_mount_root(struct
> > file_system_type *fs_type,
> > goto error_fs_info;
> > }
> >
> > - error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
> > - if (error) {
> > + device = btrfs_scan_one_device(device_name, mode, fs_type);
> > + if (IS_ERR(device)) {
> > + error = PTR_ERR(device);
> > mutex_unlock(_mutex);
> > goto error_fs_info;
> > }
> >
> > + fs_devices = device->fs_devices;
> > +
> > fs_info->fs_devices = fs_devices;
> >
> > error = btrfs_open_devices(fs_devices, mode, fs_type);
> > @@ -2227,7 +2233,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))
> > @@ -2240,19 +2246,22 @@ static long btrfs_control_ioctl(struct file *file,
> > unsigned int cmd,
> > switch (cmd) {
> > case BTRFS_IOC_SCAN_DEV:
> > mutex_lock(_mutex);
> > - ret = btrfs_scan_one_device(vol->name, FMODE_READ,
> > - _root_fs_type, _devices);
> > + device = btrfs_scan_one_device(vol->name, FMODE_READ,
> > + _root_fs_type);
> > + ret = PTR_ERR_OR_ZERO(device);
> >
Re: [PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
On Mon, Jul 16, 2018 at 05:11:17PM +0800, 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
> ---
>
> Changelog:
> v5: rebase to misc-next.
> 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 | 35 ++-
> fs/btrfs/volumes.c | 22 --
> fs/btrfs/volumes.h | 4 ++--
> 3 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index dcab4a0244e5..413bbe2d6db2 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;
>
> lockdep_assert_held(_mutex);
> @@ -918,11 +918,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, _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;
> + }
> }
> }
>
> @@ -1518,6 +1520,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;
> @@ -1561,12 +1564,15 @@ static struct dentry *btrfs_mount_root(struct
> file_system_type *fs_type,
> goto error_fs_info;
> }
>
> - error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
> - if (error) {
> + device = btrfs_scan_one_device(device_name, mode, fs_type);
> + if (IS_ERR(device)) {
> + error = PTR_ERR(device);
> mutex_unlock(_mutex);
> goto error_fs_info;
> }
>
> + fs_devices = device->fs_devices;
> +
> fs_info->fs_devices = fs_devices;
>
> error = btrfs_open_devices(fs_devices, mode, fs_type);
> @@ -2227,7 +2233,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))
> @@ -2240,19 +2246,22 @@ static long btrfs_control_ioctl(struct file *file,
> unsigned int cmd,
> switch (cmd) {
> case BTRFS_IOC_SCAN_DEV:
> mutex_lock(_mutex);
> - ret = btrfs_scan_one_device(vol->name, FMODE_READ,
> - _root_fs_type, _devices);
> + device = btrfs_scan_one_device(vol->name, FMODE_READ,
> + _root_fs_type);
> + ret = PTR_ERR_OR_ZERO(device);
> mutex_unlock(_mutex);
> break;
> case BTRFS_IOC_DEVICES_READY:
> mutex_lock(_mutex);
> - ret = btrfs_scan_one_device(vol->name, FMODE_READ,
> - _root_fs_type, _devices);
> - if (ret) {
> + device = btrfs_scan_one_device(vol->name, FMODE_READ,
> + _root_fs_type);
> + if (IS_ERR(device)) {
> + ret = PTR_ERR(device);
> mutex_unlock(_mutex);
> break;
> }
> - ret = !(fs_devices->num_devices == fs_devices->total_devices);
> + ret = !(device->fs_devices->num_devices ==
> + device->fs_devices->total_devices);
> mutex_unlock(_mutex);
> break;
> case BTRFS_IOC_GET_SUPPORTED_FEATURES:
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 88d37bfa99c8..9f485696461b 100644
>
[PATCH v5 2/2] btrfs: get device pointer from btrfs_scan_one_device
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
---
Changelog:
v5: rebase to misc-next.
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 | 35 ++-
fs/btrfs/volumes.c | 22 --
fs/btrfs/volumes.h | 4 ++--
3 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index dcab4a0244e5..413bbe2d6db2 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;
lockdep_assert_held(_mutex);
@@ -918,11 +918,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, _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;
+ }
}
}
@@ -1518,6 +1520,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;
@@ -1561,12 +1564,15 @@ static struct dentry *btrfs_mount_root(struct
file_system_type *fs_type,
goto error_fs_info;
}
- error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
- if (error) {
+ device = btrfs_scan_one_device(device_name, mode, fs_type);
+ if (IS_ERR(device)) {
+ error = PTR_ERR(device);
mutex_unlock(_mutex);
goto error_fs_info;
}
+ fs_devices = device->fs_devices;
+
fs_info->fs_devices = fs_devices;
error = btrfs_open_devices(fs_devices, mode, fs_type);
@@ -2227,7 +2233,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))
@@ -2240,19 +2246,22 @@ static long btrfs_control_ioctl(struct file *file,
unsigned int cmd,
switch (cmd) {
case BTRFS_IOC_SCAN_DEV:
mutex_lock(_mutex);
- ret = btrfs_scan_one_device(vol->name, FMODE_READ,
- _root_fs_type, _devices);
+ device = btrfs_scan_one_device(vol->name, FMODE_READ,
+ _root_fs_type);
+ ret = PTR_ERR_OR_ZERO(device);
mutex_unlock(_mutex);
break;
case BTRFS_IOC_DEVICES_READY:
mutex_lock(_mutex);
- ret = btrfs_scan_one_device(vol->name, FMODE_READ,
- _root_fs_type, _devices);
- if (ret) {
+ device = btrfs_scan_one_device(vol->name, FMODE_READ,
+ _root_fs_type);
+ if (IS_ERR(device)) {
+ ret = PTR_ERR(device);
mutex_unlock(_mutex);
break;
}
- ret = !(fs_devices->num_devices == fs_devices->total_devices);
+ ret = !(device->fs_devices->num_devices ==
+ device->fs_devices->total_devices);
mutex_unlock(_mutex);
break;
case BTRFS_IOC_GET_SUPPORTED_FEATURES:
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 88d37bfa99c8..9f485696461b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1213,15 +1213,14 @@ static int btrfs_read_disk_super(struct block_device
*bdev, u64 bytenr,
* and we are
