* [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open
@ 2017-10-20 10:33 Anand Jain
2017-10-20 10:33 ` [PATCH 2/2] btrfs: maintain consistent fmode flags for device open Anand Jain
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Anand Jain @ 2017-10-20 10:33 UTC (permalink / raw)
To: linux-btrfs
We aren't saving the fmode_t flags at %struct btrfs_device->mode same
as it used to open the device at device replace, so fix that.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/volumes.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b39737568c22..765c2bd2d8d9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
u64 tmp;
int seeding_dev = 0;
int ret = 0;
+ fmode_t flag = FMODE_WRITE | FMODE_EXCL;
if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
return -EROFS;
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
@@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 0;
- device->mode = FMODE_EXCL;
+ device->mode = flag;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
@@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
kfree(device);
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flag);
if (seeding_dev) {
mutex_unlock(&uuid_mutex);
up_write(&sb->s_umount);
@@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
struct rcu_string *name;
u64 devid = BTRFS_DEV_REPLACE_DEVID;
int ret = 0;
+ fmode_t flags = FMODE_WRITE | FMODE_EXCL;
*device_out = NULL;
if (fs_info->fs_devices->seeding) {
@@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return -EINVAL;
}
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
if (IS_ERR(bdev)) {
btrfs_err(fs_info, "target device %s is invalid!", device_path);
return PTR_ERR(bdev);
@@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 1;
- device->mode = FMODE_EXCL;
+ device->mode = flags;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
device->fs_devices = fs_info->fs_devices;
@@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return ret;
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flags);
return ret;
}
--
2.13.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] btrfs: maintain consistent fmode flags for device open
2017-10-20 10:33 [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Anand Jain
@ 2017-10-20 10:33 ` Anand Jain
2017-10-20 12:26 ` [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Nikolay Borisov
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2017-10-20 10:33 UTC (permalink / raw)
To: linux-btrfs
Devices opened through replace/add would have FMODE_WRITE | FMODE_EXCL,
as compared with devices opened through the mount which will have
FMODE_READ | FMODE_WRITE | FMODE_EXCL. So maintain consistency.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/volumes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 765c2bd2d8d9..7008b5d104d4 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2323,7 +2323,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
u64 tmp;
int seeding_dev = 0;
int ret = 0;
- fmode_t flag = FMODE_WRITE | FMODE_EXCL;
+ fmode_t flag = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
return -EROFS;
@@ -2526,7 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
struct rcu_string *name;
u64 devid = BTRFS_DEV_REPLACE_DEVID;
int ret = 0;
- fmode_t flags = FMODE_WRITE | FMODE_EXCL;
+ fmode_t flags = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
*device_out = NULL;
if (fs_info->fs_devices->seeding) {
--
2.13.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-20 10:33 [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Anand Jain
2017-10-20 10:33 ` [PATCH 2/2] btrfs: maintain consistent fmode flags for device open Anand Jain
@ 2017-10-20 12:26 ` Nikolay Borisov
2017-10-20 14:00 ` Anand Jain
2017-10-20 14:07 ` [PATCH v2 " Anand Jain
2017-11-02 6:11 ` [PATCH v3 " Anand Jain
3 siblings, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2017-10-20 12:26 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 20.10.2017 13:33, Anand Jain wrote:
> We aren't saving the fmode_t flags at %struct btrfs_device->mode same
> as it used to open the device at device replace, so fix that.
I think this could use a bit of rewording to better explain what the
problem and what it can potentially cause.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/volumes.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b39737568c22..765c2bd2d8d9 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> u64 tmp;
> int seeding_dev = 0;
> int ret = 0;
> + fmode_t flag = FMODE_WRITE | FMODE_EXCL;
>
> if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
> return -EROFS;
>
> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
> if (IS_ERR(bdev))
> return PTR_ERR(bdev);
>
> @@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> device->bdev = bdev;
> device->in_fs_metadata = 1;
> device->is_tgtdev_for_dev_replace = 0;
> - device->mode = FMODE_EXCL;
> + device->mode = flag;
So the actual bug being fixed by this patch is that currently we aren't
setting the FMODE_WRITE when initializing btrfs_device structure and
when calling blkdev_put, correct?
> device->dev_stats_valid = 1;
> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
>
> @@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
> kfree(device);
> error:
> - blkdev_put(bdev, FMODE_EXCL);
> + blkdev_put(bdev, flag);
> if (seeding_dev) {
> mutex_unlock(&uuid_mutex);
> up_write(&sb->s_umount);
> @@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> struct rcu_string *name;
> u64 devid = BTRFS_DEV_REPLACE_DEVID;
> int ret = 0;
> + fmode_t flags = FMODE_WRITE | FMODE_EXCL;
>
> *device_out = NULL;
> if (fs_info->fs_devices->seeding) {
> @@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> return -EINVAL;
> }
>
> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
> if (IS_ERR(bdev)) {
> btrfs_err(fs_info, "target device %s is invalid!", device_path);
> return PTR_ERR(bdev);
> @@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> device->bdev = bdev;
> device->in_fs_metadata = 1;
> device->is_tgtdev_for_dev_replace = 1;
> - device->mode = FMODE_EXCL;
> + device->mode = flags;
> device->dev_stats_valid = 1;
> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
> device->fs_devices = fs_info->fs_devices;
> @@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> return ret;
>
> error:
> - blkdev_put(bdev, FMODE_EXCL);
> + blkdev_put(bdev, flags);
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-20 12:26 ` [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Nikolay Borisov
@ 2017-10-20 14:00 ` Anand Jain
0 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2017-10-20 14:00 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On 10/20/2017 08:26 PM, Nikolay Borisov wrote:
>
>
> On 20.10.2017 13:33, Anand Jain wrote:
>> We aren't saving the fmode_t flags at %struct btrfs_device->mode same
>> as it used to open the device at device replace, so fix that.
>
> I think this could use a bit of rewording to better explain what the
> problem and what it can potentially cause.
OK will do it.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> fs/btrfs/volumes.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index b39737568c22..765c2bd2d8d9 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>> u64 tmp;
>> int seeding_dev = 0;
>> int ret = 0;
>> + fmode_t flag = FMODE_WRITE | FMODE_EXCL;
>>
>> if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
>> return -EROFS;
>>
>> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
>> - fs_info->bdev_holder);
>> + bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
>> if (IS_ERR(bdev))
>> return PTR_ERR(bdev);
>>
>> @@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>> device->bdev = bdev;
>> device->in_fs_metadata = 1;
>> device->is_tgtdev_for_dev_replace = 0;
>> - device->mode = FMODE_EXCL;
>> + device->mode = flag;
>
> So the actual bug being fixed by this patch is that currently we aren't
> setting the FMODE_WRITE when initializing btrfs_device structure and
> when calling blkdev_put, correct?
yes.
Thanks, Anand
>> device->dev_stats_valid = 1;
>> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
>>
>> @@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>> btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
>> kfree(device);
>> error:
>> - blkdev_put(bdev, FMODE_EXCL);
>> + blkdev_put(bdev, flag);
>> if (seeding_dev) {
>> mutex_unlock(&uuid_mutex);
>> up_write(&sb->s_umount);
>> @@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>> struct rcu_string *name;
>> u64 devid = BTRFS_DEV_REPLACE_DEVID;
>> int ret = 0;
>> + fmode_t flags = FMODE_WRITE | FMODE_EXCL;
>>
>> *device_out = NULL;
>> if (fs_info->fs_devices->seeding) {
>> @@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>> return -EINVAL;
>> }
>>
>> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
>> - fs_info->bdev_holder);
>> + bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
>> if (IS_ERR(bdev)) {
>> btrfs_err(fs_info, "target device %s is invalid!", device_path);
>> return PTR_ERR(bdev);
>> @@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>> device->bdev = bdev;
>> device->in_fs_metadata = 1;
>> device->is_tgtdev_for_dev_replace = 1;
>> - device->mode = FMODE_EXCL;
>> + device->mode = flags;
>> device->dev_stats_valid = 1;
>> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
>> device->fs_devices = fs_info->fs_devices;
>> @@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>> return ret;
>>
>> error:
>> - blkdev_put(bdev, FMODE_EXCL);
>> + blkdev_put(bdev, flags);
>> return ret;
>> }
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-20 10:33 [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Anand Jain
2017-10-20 10:33 ` [PATCH 2/2] btrfs: maintain consistent fmode flags for device open Anand Jain
2017-10-20 12:26 ` [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Nikolay Borisov
@ 2017-10-20 14:07 ` Anand Jain
2017-10-30 14:39 ` David Sterba
2017-11-02 6:11 ` [PATCH v3 " Anand Jain
3 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2017-10-20 14:07 UTC (permalink / raw)
To: linux-btrfs
We aren't setting the FMODE_WRITE when initializing btrfs_device
structure and when calling blkdev_put, however we are setting it
only when calling blkdev_get_by_path().
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Reword commit log.
fs/btrfs/volumes.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b39737568c22..765c2bd2d8d9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
u64 tmp;
int seeding_dev = 0;
int ret = 0;
+ fmode_t flag = FMODE_WRITE | FMODE_EXCL;
if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
return -EROFS;
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
@@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 0;
- device->mode = FMODE_EXCL;
+ device->mode = flag;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
@@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
kfree(device);
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flag);
if (seeding_dev) {
mutex_unlock(&uuid_mutex);
up_write(&sb->s_umount);
@@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
struct rcu_string *name;
u64 devid = BTRFS_DEV_REPLACE_DEVID;
int ret = 0;
+ fmode_t flags = FMODE_WRITE | FMODE_EXCL;
*device_out = NULL;
if (fs_info->fs_devices->seeding) {
@@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return -EINVAL;
}
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
if (IS_ERR(bdev)) {
btrfs_err(fs_info, "target device %s is invalid!", device_path);
return PTR_ERR(bdev);
@@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 1;
- device->mode = FMODE_EXCL;
+ device->mode = flags;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
device->fs_devices = fs_info->fs_devices;
@@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return ret;
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flags);
return ret;
}
--
2.13.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-20 14:07 ` [PATCH v2 " Anand Jain
@ 2017-10-30 14:39 ` David Sterba
2017-10-31 2:11 ` Anand Jain
0 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2017-10-30 14:39 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Fri, Oct 20, 2017 at 10:07:15PM +0800, Anand Jain wrote:
> We aren't setting the FMODE_WRITE when initializing btrfs_device
> structure and when calling blkdev_put, however we are setting it
> only when calling blkdev_get_by_path().
But this still does not say why this is a problem worth fixing. Nikolay
asked for it, and I would do the same, but why do we even have to ask
for that?
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-30 14:39 ` David Sterba
@ 2017-10-31 2:11 ` Anand Jain
2017-10-31 14:21 ` Nikolay Borisov
0 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2017-10-31 2:11 UTC (permalink / raw)
To: dsterba, linux-btrfs
On 10/30/2017 10:39 PM, David Sterba wrote:
> On Fri, Oct 20, 2017 at 10:07:15PM +0800, Anand Jain wrote:
>> We aren't setting the FMODE_WRITE when initializing btrfs_device
>> structure and when calling blkdev_put, however we are setting it
>> only when calling blkdev_get_by_path().
>
> But this still does not say why this is a problem worth fixing. Nikolay
> asked for it, and I would do the same, but why do we even have to ask
> for that?
Here its just a cleanup of miss match of open mode and close modes.
And there isn't any problem that I noticed.
Thanks, Anand
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
> --
> 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] 11+ messages in thread
* Re: [PATCH v2 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-31 2:11 ` Anand Jain
@ 2017-10-31 14:21 ` Nikolay Borisov
2017-11-02 5:55 ` Anand Jain
0 siblings, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2017-10-31 14:21 UTC (permalink / raw)
To: Anand Jain, dsterba, linux-btrfs
On 31.10.2017 04:11, Anand Jain wrote:
>
>
> On 10/30/2017 10:39 PM, David Sterba wrote:
>> On Fri, Oct 20, 2017 at 10:07:15PM +0800, Anand Jain wrote:
>>> We aren't setting the FMODE_WRITE when initializing btrfs_device
>>> structure and when calling blkdev_put, however we are setting it
>>> only when calling blkdev_get_by_path().
>>
>> But this still does not say why this is a problem worth fixing. Nikolay
>> asked for it, and I would do the same, but why do we even have to ask
>> for that?
>
> Here its just a cleanup of miss match of open mode and close modes.> And there isn't any problem that I noticed.
Even if that's the case, please state that explicitly in your changelog
and also put "No functional changes" if you expect it to not introduce
any change in behavior.
>
> Thanks, Anand
>
>
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>>
>> --
>> 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
>>
> --
> 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] 11+ messages in thread
* Re: [PATCH v2 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-31 14:21 ` Nikolay Borisov
@ 2017-11-02 5:55 ` Anand Jain
0 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2017-11-02 5:55 UTC (permalink / raw)
To: Nikolay Borisov, dsterba, linux-btrfs
On 10/31/2017 10:21 PM, Nikolay Borisov wrote:
>
>
> On 31.10.2017 04:11, Anand Jain wrote:
>>
>>
>> On 10/30/2017 10:39 PM, David Sterba wrote:
>>> On Fri, Oct 20, 2017 at 10:07:15PM +0800, Anand Jain wrote:
>>>> We aren't setting the FMODE_WRITE when initializing btrfs_device
>>>> structure and when calling blkdev_put, however we are setting it
>>>> only when calling blkdev_get_by_path().
>>>
>>> But this still does not say why this is a problem worth fixing. Nikolay
>>> asked for it, and I would do the same, but why do we even have to ask
>>> for that?
>>
>> Here its just a cleanup of miss match of open mode and close modes.> And there isn't any problem that I noticed.
>
> Even if that's the case, please state that explicitly in your changelog
> and also put "No functional changes" if you expect it to not introduce
> any change in behavior.
>
Ah. Will update change log.
Thanks, Anand
>>
>> Thanks, Anand
>>
>>
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>>>
>>> --
>>> 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
>>>
>> --
>> 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
>>
> --
> 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] 11+ messages in thread
* [PATCH v3 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-10-20 10:33 [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Anand Jain
` (2 preceding siblings ...)
2017-10-20 14:07 ` [PATCH v2 " Anand Jain
@ 2017-11-02 6:11 ` Anand Jain
2017-11-02 8:47 ` Nikolay Borisov
3 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2017-11-02 6:11 UTC (permalink / raw)
To: linux-btrfs
This patch maintains consistency of mode in device get and put. And is just
a cleanup patch. There isn't any problem that was noticed, so no functional
changes.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: commit update
v3: commit update
fs/btrfs/volumes.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b39737568c22..765c2bd2d8d9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
u64 tmp;
int seeding_dev = 0;
int ret = 0;
+ fmode_t flag = FMODE_WRITE | FMODE_EXCL;
if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
return -EROFS;
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
@@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 0;
- device->mode = FMODE_EXCL;
+ device->mode = flag;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
@@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
kfree(device);
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flag);
if (seeding_dev) {
mutex_unlock(&uuid_mutex);
up_write(&sb->s_umount);
@@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
struct rcu_string *name;
u64 devid = BTRFS_DEV_REPLACE_DEVID;
int ret = 0;
+ fmode_t flags = FMODE_WRITE | FMODE_EXCL;
*device_out = NULL;
if (fs_info->fs_devices->seeding) {
@@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return -EINVAL;
}
- bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
- fs_info->bdev_holder);
+ bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
if (IS_ERR(bdev)) {
btrfs_err(fs_info, "target device %s is invalid!", device_path);
return PTR_ERR(bdev);
@@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
device->bdev = bdev;
device->in_fs_metadata = 1;
device->is_tgtdev_for_dev_replace = 1;
- device->mode = FMODE_EXCL;
+ device->mode = flags;
device->dev_stats_valid = 1;
set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
device->fs_devices = fs_info->fs_devices;
@@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
return ret;
error:
- blkdev_put(bdev, FMODE_EXCL);
+ blkdev_put(bdev, flags);
return ret;
}
--
2.13.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] btrfs: match btrfs_device->mode same as it used for open
2017-11-02 6:11 ` [PATCH v3 " Anand Jain
@ 2017-11-02 8:47 ` Nikolay Borisov
0 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-02 8:47 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 2.11.2017 08:11, Anand Jain wrote:
> This patch maintains consistency of mode in device get and put. And is just
> a cleanup patch. There isn't any problem that was noticed, so no functional
> changes.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> v2: commit update
> v3: commit update
>
> fs/btrfs/volumes.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b39737568c22..765c2bd2d8d9 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2323,12 +2323,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> u64 tmp;
> int seeding_dev = 0;
> int ret = 0;
> + fmode_t flag = FMODE_WRITE | FMODE_EXCL;
>
> if (sb_rdonly(sb) && !fs_info->fs_devices->seeding)
> return -EROFS;
>
> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + bdev = blkdev_get_by_path(device_path, flag, fs_info->bdev_holder);
> if (IS_ERR(bdev))
> return PTR_ERR(bdev);
>
> @@ -2392,7 +2392,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> device->bdev = bdev;
> device->in_fs_metadata = 1;
> device->is_tgtdev_for_dev_replace = 0;
> - device->mode = FMODE_EXCL;
> + device->mode = flag;
> device->dev_stats_valid = 1;
> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
>
> @@ -2506,7 +2506,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
> btrfs_sysfs_rm_device_link(fs_info->fs_devices, device);
> kfree(device);
> error:
> - blkdev_put(bdev, FMODE_EXCL);
> + blkdev_put(bdev, flag);
> if (seeding_dev) {
> mutex_unlock(&uuid_mutex);
> up_write(&sb->s_umount);
> @@ -2526,6 +2526,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> struct rcu_string *name;
> u64 devid = BTRFS_DEV_REPLACE_DEVID;
> int ret = 0;
> + fmode_t flags = FMODE_WRITE | FMODE_EXCL;
>
> *device_out = NULL;
> if (fs_info->fs_devices->seeding) {
> @@ -2533,8 +2534,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> return -EINVAL;
> }
>
> - bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL,
> - fs_info->bdev_holder);
> + bdev = blkdev_get_by_path(device_path, flags, fs_info->bdev_holder);
> if (IS_ERR(bdev)) {
> btrfs_err(fs_info, "target device %s is invalid!", device_path);
> return PTR_ERR(bdev);
> @@ -2595,7 +2595,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> device->bdev = bdev;
> device->in_fs_metadata = 1;
> device->is_tgtdev_for_dev_replace = 1;
> - device->mode = FMODE_EXCL;
> + device->mode = flags;
> device->dev_stats_valid = 1;
> set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
> device->fs_devices = fs_info->fs_devices;
> @@ -2608,7 +2608,7 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
> return ret;
>
> error:
> - blkdev_put(bdev, FMODE_EXCL);
> + blkdev_put(bdev, flags);
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-11-02 8:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 10:33 [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Anand Jain
2017-10-20 10:33 ` [PATCH 2/2] btrfs: maintain consistent fmode flags for device open Anand Jain
2017-10-20 12:26 ` [PATCH 1/2] btrfs: match btrfs_device->mode same as it used for open Nikolay Borisov
2017-10-20 14:00 ` Anand Jain
2017-10-20 14:07 ` [PATCH v2 " Anand Jain
2017-10-30 14:39 ` David Sterba
2017-10-31 2:11 ` Anand Jain
2017-10-31 14:21 ` Nikolay Borisov
2017-11-02 5:55 ` Anand Jain
2017-11-02 6:11 ` [PATCH v3 " Anand Jain
2017-11-02 8:47 ` Nikolay Borisov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).