* [PATCH 0/3] fix bug in btrfs_init_new_device()
@ 2017-09-25 12:31 Anand Jain
2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Anand Jain @ 2017-09-25 12:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
1/3 fixes a bug which failed to reset writable when sprouting failed
2/3 is cleanup and preparatory to fix 3/3
3/3 fixes BUG_ON in btrfs_init_new_device()
Anand Jain (3):
btrfs: undo writable when sprouting fails
btrfs: cleanup btrfs_init_new_device()
btrfs: fix BUG_ON in btrfs_init_new_device()
fs/btrfs/volumes.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
--
2.13.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] btrfs: undo writable when sprouting fails 2017-09-25 12:31 [PATCH 0/3] fix bug in btrfs_init_new_device() Anand Jain @ 2017-09-25 12:31 ` Anand Jain 2017-09-25 13:03 ` Qu Wenruo 2017-09-25 14:52 ` David Sterba 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain ` (2 subsequent siblings) 3 siblings, 2 replies; 18+ messages in thread From: Anand Jain @ 2017-09-25 12:31 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba 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] 18+ messages in thread
* Re: [PATCH 1/3] btrfs: undo writable when sprouting fails 2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain @ 2017-09-25 13:03 ` Qu Wenruo 2017-09-25 14:52 ` David Sterba 1 sibling, 0 replies; 18+ messages in thread From: Qu Wenruo @ 2017-09-25 13:03 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 2017年09月25日 20:31, Anand Jain wrote: > Signed-off-by: Anand Jain <anand.jain@oracle.com> The error_trans tag needs to revert the RDONLY flag without doubt. But just a small question. Just lines before error_trans tag, we're committing transaction and then relocating system chunks. In which we don't use goto error branch but returning directly. Don't we need to to revert the RDONLY flag in that error handler? Thanks, Qu > --- > 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); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] btrfs: undo writable when sprouting fails 2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain 2017-09-25 13:03 ` Qu Wenruo @ 2017-09-25 14:52 ` David Sterba 1 sibling, 0 replies; 18+ messages in thread From: David Sterba @ 2017-09-25 14:52 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, dsterba On Mon, Sep 25, 2017 at 08:31:02PM +0800, Anand Jain wrote: > Signed-off-by: Anand Jain <anand.jain@oracle.com> This does not look like change trivial enough to just leave out the description. Please explain in more detail. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 12:31 [PATCH 0/3] fix bug in btrfs_init_new_device() Anand Jain 2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain @ 2017-09-25 12:31 ` Anand Jain 2017-09-25 13:07 ` Nikolay Borisov ` (2 more replies) 2017-09-25 12:31 ` [PATCH 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain 3 siblings, 3 replies; 18+ messages in thread From: Anand Jain @ 2017-09-25 12:31 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba Moves btrfs_abort_transaction() to the error goto and renames error_trans to error_sysfs. This is a preparatory patch to remove the BUG_ON() in btrfs_init_new_device(). Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9d64700cc9b6..5c34eea9d12d 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2443,26 +2443,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path mutex_lock(&fs_info->chunk_mutex); ret = init_first_rw_device(trans, fs_info); mutex_unlock(&fs_info->chunk_mutex); - if (ret) { - btrfs_abort_transaction(trans, ret); - goto error_trans; - } + if (ret) + goto error_sysfs; } ret = btrfs_add_device(trans, fs_info, device); - if (ret) { - btrfs_abort_transaction(trans, ret); - goto error_trans; - } + if (ret) + goto error_sysfs; if (seeding_dev) { char fsid_buf[BTRFS_UUID_UNPARSED_SIZE]; ret = btrfs_finish_sprout(trans, fs_info); - if (ret) { - btrfs_abort_transaction(trans, ret); - goto error_trans; - } + if (ret) + goto error_sysfs; /* Sprouting would change fsid of the mounted root, * so rename the fsid on the sysfs @@ -2500,12 +2494,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path update_dev_time(device_path); return ret; -error_trans: +error_sysfs: + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); if (seeding_dev) sb->s_flags |= MS_RDONLY; + btrfs_abort_transaction(trans, ret); 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] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain @ 2017-09-25 13:07 ` Nikolay Borisov 2017-09-25 13:09 ` Nikolay Borisov 2017-09-25 13:08 ` Qu Wenruo 2017-09-25 14:56 ` David Sterba 2 siblings, 1 reply; 18+ messages in thread From: Nikolay Borisov @ 2017-09-25 13:07 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 25.09.2017 15:31, Anand Jain wrote: > Moves btrfs_abort_transaction() to the error goto and renames > error_trans to error_sysfs. This is a preparatory patch to > remove the BUG_ON() in btrfs_init_new_device(). > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/volumes.c | 23 +++++++++-------------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 9d64700cc9b6..5c34eea9d12d 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2443,26 +2443,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > mutex_lock(&fs_info->chunk_mutex); > ret = init_first_rw_device(trans, fs_info); > mutex_unlock(&fs_info->chunk_mutex); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > } > > ret = btrfs_add_device(trans, fs_info, device); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > > if (seeding_dev) { > char fsid_buf[BTRFS_UUID_UNPARSED_SIZE]; > > ret = btrfs_finish_sprout(trans, fs_info); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > > /* Sprouting would change fsid of the mounted root, > * so rename the fsid on the sysfs > @@ -2500,12 +2494,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > update_dev_time(device_path); > return ret; > > -error_trans: > +error_sysfs: > + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); Any reason why you move btrfs_sysfs_rm_device_link? > if (seeding_dev) > sb->s_flags |= MS_RDONL> + btrfs_abort_transaction(trans, ret); > 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); Also which branch is your code based on since in Linus' master the error handling (before your patch) looks a bit different. E.g. the MS_RDONL stuff look wrong. error_trans: 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); if (seeding_dev) { mutex_unlock(&uuid_mutex); up_write(&sb->s_umount); } return ret; > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 13:07 ` Nikolay Borisov @ 2017-09-25 13:09 ` Nikolay Borisov 2017-09-26 7:57 ` Anand Jain 0 siblings, 1 reply; 18+ messages in thread From: Nikolay Borisov @ 2017-09-25 13:09 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 25.09.2017 16:07, Nikolay Borisov wrote: > > > On 25.09.2017 15:31, Anand Jain wrote: >> Moves btrfs_abort_transaction() to the error goto and renames >> error_trans to error_sysfs. This is a preparatory patch to >> remove the BUG_ON() in btrfs_init_new_device(). >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> fs/btrfs/volumes.c | 23 +++++++++-------------- >> 1 file changed, 9 insertions(+), 14 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 9d64700cc9b6..5c34eea9d12d 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -2443,26 +2443,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path >> mutex_lock(&fs_info->chunk_mutex); >> ret = init_first_rw_device(trans, fs_info); >> mutex_unlock(&fs_info->chunk_mutex); >> - if (ret) { >> - btrfs_abort_transaction(trans, ret); >> - goto error_trans; >> - } >> + if (ret) >> + goto error_sysfs; >> } >> >> ret = btrfs_add_device(trans, fs_info, device); >> - if (ret) { >> - btrfs_abort_transaction(trans, ret); >> - goto error_trans; >> - } >> + if (ret) >> + goto error_sysfs; >> >> if (seeding_dev) { >> char fsid_buf[BTRFS_UUID_UNPARSED_SIZE]; >> >> ret = btrfs_finish_sprout(trans, fs_info); >> - if (ret) { >> - btrfs_abort_transaction(trans, ret); >> - goto error_trans; >> - } >> + if (ret) >> + goto error_sysfs; >> >> /* Sprouting would change fsid of the mounted root, >> * so rename the fsid on the sysfs >> @@ -2500,12 +2494,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path >> update_dev_time(device_path); >> return ret; >> >> -error_trans: >> +error_sysfs: >> + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); > > Any reason why you move btrfs_sysfs_rm_device_link? > >> if (seeding_dev) >> sb->s_flags |= MS_RDONL> + btrfs_abort_transaction(trans, ret); >> 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); > > Also which branch is your code based on since in Linus' master the error > handling (before your patch) looks a bit different. E.g. the MS_RDONL > stuff look wrong. > > error_trans: > > 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); > > if (seeding_dev) { > > mutex_unlock(&uuid_mutex); > > up_write(&sb->s_umount); > > } > > return ret; Ok, turned out I hadn't received 1/3 so this answers this question. The sysfs rm stands though > >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 13:09 ` Nikolay Borisov @ 2017-09-26 7:57 ` Anand Jain 2017-09-26 8:00 ` Anand Jain 2017-09-26 8:07 ` Nikolay Borisov 0 siblings, 2 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 7:57 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: dsterba > The > sysfs rm stands though This is a preparatory patch for 3/3, at BUG_ON it needs an exit without the btrfs_sysfs_add_device_link() part. -Anand ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-26 7:57 ` Anand Jain @ 2017-09-26 8:00 ` Anand Jain 2017-09-26 8:07 ` Nikolay Borisov 1 sibling, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 8:00 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: dsterba On 09/26/2017 03:57 PM, Anand Jain wrote: > >> The >> sysfs rm stands though > > This is a preparatory patch for 3/3, at BUG_ON it needs an exit > without the btrfs_sysfs_add_device_link() part. typo s/btrfs_sysfs_add_device_link/btrfs_sysfs_rm_device_link/ > -Anand > -- > 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] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-26 7:57 ` Anand Jain 2017-09-26 8:00 ` Anand Jain @ 2017-09-26 8:07 ` Nikolay Borisov 2017-09-26 8:45 ` Anand Jain 1 sibling, 1 reply; 18+ messages in thread From: Nikolay Borisov @ 2017-09-26 8:07 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 26.09.2017 10:57, Anand Jain wrote: > >> The >> sysfs rm stands though > > This is a preparatory patch for 3/3, at BUG_ON it needs an exit > without the btrfs_sysfs_add_device_link() part. In this case I'd rather you have this change in 3/3, because on its own it doesn't really make sense and cannot easily be figured out if doing git blame. > > -Anand > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-26 8:07 ` Nikolay Borisov @ 2017-09-26 8:45 ` Anand Jain 0 siblings, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 8:45 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: dsterba >>> The >>> sysfs rm stands though >> >> This is a preparatory patch for 3/3, at BUG_ON it needs an exit >> without the btrfs_sysfs_add_device_link() part. > > In this case I'd rather you have this change in 3/3, because on its own > it doesn't really make sense and cannot easily be figured out if doing > git blame. Fixed in v3. Thanks for suggesting. -Anand ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain 2017-09-25 13:07 ` Nikolay Borisov @ 2017-09-25 13:08 ` Qu Wenruo 2017-09-25 14:56 ` David Sterba 2 siblings, 0 replies; 18+ messages in thread From: Qu Wenruo @ 2017-09-25 13:08 UTC (permalink / raw) To: Anand Jain, linux-btrfs; +Cc: dsterba On 2017年09月25日 20:31, Anand Jain wrote: > Moves btrfs_abort_transaction() to the error goto and renames > error_trans to error_sysfs. This is a preparatory patch to > remove the BUG_ON() in btrfs_init_new_device(). > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/volumes.c | 23 +++++++++-------------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 9d64700cc9b6..5c34eea9d12d 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -2443,26 +2443,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > mutex_lock(&fs_info->chunk_mutex); > ret = init_first_rw_device(trans, fs_info); > mutex_unlock(&fs_info->chunk_mutex); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > } > > ret = btrfs_add_device(trans, fs_info, device); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > > if (seeding_dev) { > char fsid_buf[BTRFS_UUID_UNPARSED_SIZE]; > > ret = btrfs_finish_sprout(trans, fs_info); > - if (ret) { > - btrfs_abort_transaction(trans, ret); > - goto error_trans; > - } > + if (ret) > + goto error_sysfs; > > /* Sprouting would change fsid of the mounted root, > * so rename the fsid on the sysfs > @@ -2500,12 +2494,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path > update_dev_time(device_path); > return ret; > > -error_trans: > +error_sysfs: > + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); > if (seeding_dev) > sb->s_flags |= MS_RDONLY; > + btrfs_abort_transaction(trans, ret); > btrfs_end_transaction(trans); Abort then end? Looks redundant at least. > rcu_string_free(device->name); > - btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); Any particular reason to move this line ahead? I didn't see it has something to do with transaction, so just curious about the purpose. Thanks, Qu > kfree(device); > error: > blkdev_put(bdev, FMODE_EXCL); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain 2017-09-25 13:07 ` Nikolay Borisov 2017-09-25 13:08 ` Qu Wenruo @ 2017-09-25 14:56 ` David Sterba 2 siblings, 0 replies; 18+ messages in thread From: David Sterba @ 2017-09-25 14:56 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, dsterba On Mon, Sep 25, 2017 at 08:31:03PM +0800, Anand Jain wrote: > Moves btrfs_abort_transaction() to the error goto and renames > error_trans to error_sysfs. This is a preparatory patch to > remove the BUG_ON() in btrfs_init_new_device(). Please keep all transaction abort exactly at the place where they happened and do not merge them to one. This pattern should be used everwhere and is important when debugging because we can pinpoint the line in the code from the syslog message and do not have to guess which way it got to the merged call. This makes the binary large, looks like a code duplication in the code but for a good reason. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() 2017-09-25 12:31 [PATCH 0/3] fix bug in btrfs_init_new_device() Anand Jain 2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain @ 2017-09-25 12:31 ` Anand Jain 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain 3 siblings, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-25 12:31 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba Here we will simply return the error to the caller. And handle the fail condition by calling the abort transaction through the error path. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 5c34eea9d12d..93eaf314fe72 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2399,7 +2399,8 @@ 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) + goto error_trans; } device->fs_devices = fs_info->fs_devices; @@ -2496,6 +2497,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path error_sysfs: btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); +error_trans: if (seeding_dev) sb->s_flags |= MS_RDONLY; btrfs_abort_transaction(trans, ret); -- 2.13.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 0/3] fix bug in btrfs_init_new_device() 2017-09-25 12:31 [PATCH 0/3] fix bug in btrfs_init_new_device() Anand Jain ` (2 preceding siblings ...) 2017-09-25 12:31 ` [PATCH 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain @ 2017-09-26 7:48 ` Anand Jain 2017-09-26 7:48 ` [PATCH v2 1/3] btrfs: undo writable when sprouting fails Anand Jain ` (2 more replies) 3 siblings, 3 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 7:48 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba 1/3 fixes a bug which failed to reset writable when sprouting failed 2/3 is cleanup and preparatory to fix 3/3 3/3 fixes BUG_ON in btrfs_init_new_device() v2: add commit log to 1/3 do not consolidate btrfs_abort_transaction() in 2/3 Anand Jain (3): btrfs: undo writable when sprouting fails btrfs: cleanup btrfs_init_new_device() 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] 18+ messages in thread
* [PATCH v2 1/3] btrfs: undo writable when sprouting fails 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain @ 2017-09-26 7:48 ` Anand Jain 2017-09-26 7:48 ` [PATCH v2 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain 2017-09-26 7:48 ` [PATCH v2 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 2 siblings, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 7:48 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> --- v2: add commit log 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] 18+ messages in thread
* [PATCH v2 2/3] btrfs: cleanup btrfs_init_new_device() 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain 2017-09-26 7:48 ` [PATCH v2 1/3] btrfs: undo writable when sprouting fails Anand Jain @ 2017-09-26 7:48 ` Anand Jain 2017-09-26 7:48 ` [PATCH v2 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 2 siblings, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 7:48 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba Moves btrfs_abort_transaction() to the error goto and renames error_trans to error_sysfs. This is a preparatory patch to remove the BUG_ON() in btrfs_init_new_device(). Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9d64700cc9b6..22eb81794375 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2445,14 +2445,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 +2461,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 +2500,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path update_dev_time(device_path); return ret; -error_trans: +error_sysfs: + btrfs_sysfs_rm_device_link(fs_info->fs_devices, device); 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] 18+ messages in thread
* [PATCH v2 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain 2017-09-26 7:48 ` [PATCH v2 1/3] btrfs: undo writable when sprouting fails Anand Jain 2017-09-26 7:48 ` [PATCH v2 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain @ 2017-09-26 7:48 ` Anand Jain 2 siblings, 0 replies; 18+ messages in thread From: Anand Jain @ 2017-09-26 7:48 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 through the error path. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v2: do not consolidate btrfs_abort_transaction() fs/btrfs/volumes.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 22eb81794375..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; @@ -2502,6 +2505,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path 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); -- 2.13.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-09-26 8:37 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-25 12:31 [PATCH 0/3] fix bug in btrfs_init_new_device() Anand Jain 2017-09-25 12:31 ` [PATCH 1/3] btrfs: undo writable when sprouting fails Anand Jain 2017-09-25 13:03 ` Qu Wenruo 2017-09-25 14:52 ` David Sterba 2017-09-25 12:31 ` [PATCH 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain 2017-09-25 13:07 ` Nikolay Borisov 2017-09-25 13:09 ` Nikolay Borisov 2017-09-26 7:57 ` Anand Jain 2017-09-26 8:00 ` Anand Jain 2017-09-26 8:07 ` Nikolay Borisov 2017-09-26 8:45 ` Anand Jain 2017-09-25 13:08 ` Qu Wenruo 2017-09-25 14:56 ` David Sterba 2017-09-25 12:31 ` [PATCH 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() Anand Jain 2017-09-26 7:48 ` [PATCH v2 0/3] fix bug " Anand Jain 2017-09-26 7:48 ` [PATCH v2 1/3] btrfs: undo writable when sprouting fails Anand Jain 2017-09-26 7:48 ` [PATCH v2 2/3] btrfs: cleanup btrfs_init_new_device() Anand Jain 2017-09-26 7:48 ` [PATCH v2 3/3] btrfs: fix BUG_ON in btrfs_init_new_device() 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).