* [PATCH v3 0/2] fix bug in btrfs_init_new_device() @ 2017-09-26 8:41 Anand Jain 2017-09-26 8:41 ` [PATCH v3 1/2] btrfs: undo writable when sprouting fails Anand Jain 2017-09-26 8:41 ` [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 0 siblings, 2 replies; 8+ messages in thread From: Anand Jain @ 2017-09-26 8:41 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba 1/2 fixes a bug which failed to reset writable when sprouting failed 2/2 fixes BUG_ON in btrfs_init_new_device() Anand Jain (2): btrfs: undo writable when sprouting fails btrfs: fix BUG_ON in btrfs_init_new_device() fs/btrfs/volumes.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) -- 2.13.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] btrfs: undo writable when sprouting fails 2017-09-26 8:41 [PATCH v3 0/2] fix bug in btrfs_init_new_device() Anand Jain @ 2017-09-26 8:41 ` Anand Jain 2017-09-26 12:57 ` Qu Wenruo 2017-09-26 8:41 ` [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 1 sibling, 1 reply; 8+ messages in thread From: Anand Jain @ 2017-09-26 8:41 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba When new device is being added to seed FS, seed FS is marked writable, but when we fail to bring in the new device, we missed to undo the writable part. This patch fixes it. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 0e8f16c305df..9d64700cc9b6 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path return ret; error_trans: + if (seeding_dev) + sb->s_flags |= MS_RDONLY; btrfs_end_transaction(trans); rcu_string_free(device->name); btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); -- 2.13.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] btrfs: undo writable when sprouting fails 2017-09-26 8:41 ` [PATCH v3 1/2] btrfs: undo writable when sprouting fails Anand Jain @ 2017-09-26 12:57 ` Qu Wenruo 2017-09-26 17:49 ` David Sterba 0 siblings, 1 reply; 8+ messages in thread From: Qu Wenruo @ 2017-09-26 12:57 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 2017年09月26日 16:41, Anand Jain wrote: > When new device is being added to seed FS, seed FS is marked writable, > but when we fail to bring in the new device, we missed to undo the > writable part. This patch fixes it. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/volumes.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 0e8f16c305df..9d64700cc9b6 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > return ret; > > error_trans: > + if (seeding_dev) > + sb->s_flags |= MS_RDONLY; Still the same question. --- if (seeding_dev) { mutex_unlock(&uuid_mutex); up_write(&sb->s_umount); if (ret) /* transaction commit */ return ret; ret = btrfs_relocate_sys_chunks(fs_info); if (ret < 0) btrfs_handle_fs_error(fs_info, ret, "Failed to relocate sys chunks after device initialization. This can be fixed using the \"btrfs balance\" command."); trans = btrfs_attach_transaction(root); if (IS_ERR(trans)) { if (PTR_ERR(trans) == -ENOENT) return 0; return PTR_ERR(trans); } --- In the above btrfs_attch_transaction() error handler, which just returned without going to error_trans tags, did we misseed the s_flag revert thing? Thanks, Qu > btrfs_end_transaction(trans); > rcu_string_free(device->name); > btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] btrfs: undo writable when sprouting fails 2017-09-26 12:57 ` Qu Wenruo @ 2017-09-26 17:49 ` David Sterba 2017-09-27 9:58 ` Anand Jain 0 siblings, 1 reply; 8+ messages in thread From: David Sterba @ 2017-09-26 17:49 UTC (permalink / raw) To: Qu Wenruo; +Cc: Anand Jain, linux-btrfs, dsterba On Tue, Sep 26, 2017 at 08:57:47PM +0800, Qu Wenruo wrote: > > > On 2017年09月26日 16:41, Anand Jain wrote: > > When new device is being added to seed FS, seed FS is marked writable, > > but when we fail to bring in the new device, we missed to undo the > > writable part. This patch fixes it. > > > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > > --- > > fs/btrfs/volumes.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > index 0e8f16c305df..9d64700cc9b6 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > > return ret; > > > > error_trans: > > + if (seeding_dev) > > + sb->s_flags |= MS_RDONLY; > > Still the same question. > > --- > if (seeding_dev) { > mutex_unlock(&uuid_mutex); > up_write(&sb->s_umount); > > if (ret) /* transaction commit */ > return ret; > > ret = btrfs_relocate_sys_chunks(fs_info); > if (ret < 0) > btrfs_handle_fs_error(fs_info, ret, > "Failed to relocate sys chunks after device initialization. This > can be fixed using the \"btrfs balance\" command."); > trans = btrfs_attach_transaction(root); > if (IS_ERR(trans)) { > if (PTR_ERR(trans) == -ENOENT) > return 0; > return PTR_ERR(trans); > } > --- > In the above btrfs_attch_transaction() error handler, which just > returned without going to error_trans tags, did we misseed the s_flag > revert thing? I think so. Also I think "sb->s_flags |= MS_RDONLY;" can be moved to the 'if' after the error: label, where we already check the 'seeding_dev' condition. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] btrfs: undo writable when sprouting fails 2017-09-26 17:49 ` David Sterba @ 2017-09-27 9:58 ` Anand Jain 0 siblings, 0 replies; 8+ messages in thread From: Anand Jain @ 2017-09-27 9:58 UTC (permalink / raw) To: dsterba, Qu Wenruo, linux-btrfs On 09/27/2017 01:49 AM, David Sterba wrote: > On Tue, Sep 26, 2017 at 08:57:47PM +0800, Qu Wenruo wrote: >> >> >> On 2017年09月26日 16:41, Anand Jain wrote: >>> When new device is being added to seed FS, seed FS is marked writable, >>> but when we fail to bring in the new device, we missed to undo the >>> writable part. This patch fixes it. >>> >>> Signed-off-by: Anand Jain <anand.jain@oracle.com> >>> --- >>> fs/btrfs/volumes.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index 0e8f16c305df..9d64700cc9b6 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -2501,6 +2501,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path >>> return ret; >>> >>> error_trans: >>> + if (seeding_dev) >>> + sb->s_flags |= MS_RDONLY; >> >> Still the same question. >> >> --- >> if (seeding_dev) { >> mutex_unlock(&uuid_mutex); >> up_write(&sb->s_umount); >> >> if (ret) /* transaction commit */ >> return ret; >> >> ret = btrfs_relocate_sys_chunks(fs_info); >> if (ret < 0) >> btrfs_handle_fs_error(fs_info, ret, >> "Failed to relocate sys chunks after device initialization. This >> can be fixed using the \"btrfs balance\" command."); >> trans = btrfs_attach_transaction(root); >> if (IS_ERR(trans)) { >> if (PTR_ERR(trans) == -ENOENT) >> return 0; >> return PTR_ERR(trans); >> } >> --- >> In the above btrfs_attch_transaction() error handler, which just >> returned without going to error_trans tags, did we misseed the s_flag >> revert thing? > > I think so. Also I think "sb->s_flags |= MS_RDONLY;" can be moved to the > 'if' after the error: label, where we already check the 'seeding_dev' > condition. Qu, David, Thanks. I have fixed that in a new patch as its wasn't about fixing the BUG_ON(). Further, there is another bug that, in the original code we failed to undo the btrfs_prepare_sprout() part. I am attempting to fix that, its bit complicated. For now, fix for Qu found bug is sent to the ML. -Anand ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() 2017-09-26 8:41 [PATCH v3 0/2] fix bug in btrfs_init_new_device() Anand Jain 2017-09-26 8:41 ` [PATCH v3 1/2] btrfs: undo writable when sprouting fails Anand Jain @ 2017-09-26 8:41 ` Anand Jain 2017-09-26 12:58 ` Qu Wenruo 1 sibling, 1 reply; 8+ messages in thread From: Anand Jain @ 2017-09-26 8:41 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba Instead of BUG_ON return error to the caller. And handle the fail condition by calling the abort transaction and going through the error path. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9d64700cc9b6..4cb575fbf643 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2399,7 +2399,10 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path if (seeding_dev) { sb->s_flags &= ~MS_RDONLY; ret = btrfs_prepare_sprout(fs_info); - BUG_ON(ret); /* -ENOMEM */ + if (ret) { + btrfs_abort_transaction(trans, ret); + goto error_trans; + } } device->fs_devices = fs_info->fs_devices; @@ -2445,14 +2448,14 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path mutex_unlock(&fs_info->chunk_mutex); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } } ret = btrfs_add_device(trans, fs_info, device); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } if (seeding_dev) { @@ -2461,7 +2464,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path ret = btrfs_finish_sprout(trans, fs_info); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } /* Sprouting would change fsid of the mounted root, @@ -2500,12 +2503,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path update_dev_time(device_path); return ret; +error_sysfs: + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); error_trans: if (seeding_dev) sb->s_flags |= MS_RDONLY; btrfs_end_transaction(trans); rcu_string_free(device->name); - btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); kfree(device); error: blkdev_put(bdev, FMODE_EXCL); -- 2.13.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() 2017-09-26 8:41 ` [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain @ 2017-09-26 12:58 ` Qu Wenruo 0 siblings, 0 replies; 8+ messages in thread From: Qu Wenruo @ 2017-09-26 12:58 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 2017年09月26日 16:41, Anand Jain wrote: > Instead of BUG_ON return error to the caller. And handle the fail > condition by calling the abort transaction and going through the > error path. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com> Thanks, Qu > --- > fs/btrfs/volumes.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 9d64700cc9b6..4cb575fbf643 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2399,7 +2399,10 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > if (seeding_dev) { > sb->s_flags &= ~MS_RDONLY; > ret = btrfs_prepare_sprout(fs_info); > - BUG_ON(ret); /* -ENOMEM */ > + if (ret) { > + btrfs_abort_transaction(trans, ret); > + goto error_trans; > + } > } > > device->fs_devices = fs_info->fs_devices; > @@ -2445,14 +2448,14 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > mutex_unlock(&fs_info->chunk_mutex); > if (ret) { > btrfs_abort_transaction(trans, ret); > - goto error_trans; > + goto error_sysfs; > } > } > > ret = btrfs_add_device(trans, fs_info, device); > if (ret) { > btrfs_abort_transaction(trans, ret); > - goto error_trans; > + goto error_sysfs; > } > > if (seeding_dev) { > @@ -2461,7 +2464,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > ret = btrfs_finish_sprout(trans, fs_info); > if (ret) { > btrfs_abort_transaction(trans, ret); > - goto error_trans; > + goto error_sysfs; > } > > /* Sprouting would change fsid of the mounted root, > @@ -2500,12 +2503,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > update_dev_time(device_path); > return ret; > > +error_sysfs: > + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); > error_trans: > if (seeding_dev) > sb->s_flags |= MS_RDONLY; > btrfs_end_transaction(trans); > rcu_string_free(device->name); > - btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); > kfree(device); > error: > blkdev_put(bdev, FMODE_EXCL); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/2] fix bug in btrfs_init_new_device() @ 2017-09-26 8:47 Anand Jain 2017-09-26 8:47 ` [PATCH v3 2/2] btrfs: fix BUG_ON " Anand Jain 0 siblings, 1 reply; 8+ messages in thread From: Anand Jain @ 2017-09-26 8:47 UTC (permalink / raw) To: linux-btrfs 1/2 fixes a bug which failed to reset writable when sprouting failed 2/2 fixes BUG_ON in btrfs_init_new_device() Anand Jain (2): btrfs: undo writable when sprouting fails btrfs: fix BUG_ON in btrfs_init_new_device() fs/btrfs/volumes.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) -- 2.13.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() 2017-09-26 8:47 [PATCH v3 0/2] fix bug " Anand Jain @ 2017-09-26 8:47 ` Anand Jain 0 siblings, 0 replies; 8+ messages in thread From: Anand Jain @ 2017-09-26 8:47 UTC (permalink / raw) To: linux-btrfs Instead of BUG_ON return error to the caller. And handle the fail condition by calling the abort transaction and going through the error path. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v2: do not consolidate btrfs_abort_transaction() v3: meld 2/3 and 3/3 from v2 fs/btrfs/volumes.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9d64700cc9b6..4cb575fbf643 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2399,7 +2399,10 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path if (seeding_dev) { sb->s_flags &= ~MS_RDONLY; ret = btrfs_prepare_sprout(fs_info); - BUG_ON(ret); /* -ENOMEM */ + if (ret) { + btrfs_abort_transaction(trans, ret); + goto error_trans; + } } device->fs_devices = fs_info->fs_devices; @@ -2445,14 +2448,14 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path mutex_unlock(&fs_info->chunk_mutex); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } } ret = btrfs_add_device(trans, fs_info, device); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } if (seeding_dev) { @@ -2461,7 +2464,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path ret = btrfs_finish_sprout(trans, fs_info); if (ret) { btrfs_abort_transaction(trans, ret); - goto error_trans; + goto error_sysfs; } /* Sprouting would change fsid of the mounted root, @@ -2500,12 +2503,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path update_dev_time(device_path); return ret; +error_sysfs: + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); error_trans: if (seeding_dev) sb->s_flags |= MS_RDONLY; btrfs_end_transaction(trans); rcu_string_free(device->name); - btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); kfree(device); error: blkdev_put(bdev, FMODE_EXCL); -- 2.13.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-27 9:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-26 8:41 [PATCH v3 0/2] fix bug in btrfs_init_new_device() Anand Jain 2017-09-26 8:41 ` [PATCH v3 1/2] btrfs: undo writable when sprouting fails Anand Jain 2017-09-26 12:57 ` Qu Wenruo 2017-09-26 17:49 ` David Sterba 2017-09-27 9:58 ` Anand Jain 2017-09-26 8:41 ` [PATCH v3 2/2] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 2017-09-26 12:58 ` Qu Wenruo -- strict thread matches above, loose matches on Subject: below -- 2017-09-26 8:47 [PATCH v3 0/2] fix bug " Anand Jain 2017-09-26 8:47 ` [PATCH v3 2/2] btrfs: fix BUG_ON " Anand Jain
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).