| Message ID | 20191113102728.8835-1-jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2019/11/13 下午6:27, Johannes Thumshirn wrote: > This series attempts to remove the BUG_ON()s in btrfs_close_one_device(). > Therefore some reorganization of btrfs_close_one_device() and > close_fs_devices() was needed. > > Forthermore a new BUG_ON() had to be temporarily introduced but is removed > again in the last patch of theis series. > > Although it is generally legal to return -ENOMEM on umount(2), the error > handling up until close_ctree() as neither close_ctree() nor btrfs_put_super() > would be able to handle the error. > > This series has passed fstests without any deviation from the baseline and > also the new error handling was tested via error injection using this snippet: > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 7c55169c0613..c58802c9c39c 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1069,6 +1069,9 @@ static int btrfs_close_one_device(struct btrfs_device *device) > > new_device = btrfs_alloc_device(NULL, &device->devid, > device->uuid); > + btrfs_free_device(new_device); > + pr_err("%s() INJECTING -ENOMEM\n", __func__); > + new_device = ERR_PTR(-ENOMEM); > if (IS_ERR(new_device)) > return PTR_ERR(new_device); > > Changes to v1: > > Fixed the decremt of btrfs_fs_devices::seeding. > > In addition to this, I've added two patches changing btrfs_fs_devices::seeding > and btrfs_fs_devices::rotating to bool, as they are in fact used as booleans. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > > Johannes Thumshirn (7): > btrfs: decrement number of open devices after closing the device not > before > btrfs: handle device allocation failure in btrfs_close_one_device() > btrfs: handle allocation failure in strdup > btrfs: handle error return of close_fs_devices() > btrfs: remove final BUG_ON() in close_fs_devices() > btrfs: change btrfs_fs_devices::seeing to bool > btrfs: change btrfs_fs_devices::rotating to bool > > fs/btrfs/volumes.c | 81 ++++++++++++++++++++++++++++++++++++------------------ > fs/btrfs/volumes.h | 4 +-- > 2 files changed, 56 insertions(+), 29 deletions(-) >
On Wed, Nov 13, 2019 at 11:27:21AM +0100, Johannes Thumshirn wrote: > This series attempts to remove the BUG_ON()s in btrfs_close_one_device(). > Therefore some reorganization of btrfs_close_one_device() and > close_fs_devices() was needed. > > Forthermore a new BUG_ON() had to be temporarily introduced but is removed > again in the last patch of theis series. > > Although it is generally legal to return -ENOMEM on umount(2), the error > handling up until close_ctree() as neither close_ctree() nor btrfs_put_super() > would be able to handle the error. > > This series has passed fstests without any deviation from the baseline and > also the new error handling was tested via error injection using this snippet: > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 7c55169c0613..c58802c9c39c 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1069,6 +1069,9 @@ static int btrfs_close_one_device(struct btrfs_device *device) > > new_device = btrfs_alloc_device(NULL, &device->devid, > device->uuid); > + btrfs_free_device(new_device); > + pr_err("%s() INJECTING -ENOMEM\n", __func__); > + new_device = ERR_PTR(-ENOMEM); > if (IS_ERR(new_device)) > return PTR_ERR(new_device); > > Changes to v1: > > Fixed the decremt of btrfs_fs_devices::seeding. > > In addition to this, I've added two patches changing btrfs_fs_devices::seeding > and btrfs_fs_devices::rotating to bool, as they are in fact used as booleans. > > Johannes Thumshirn (7): > btrfs: decrement number of open devices after closing the device not > before > btrfs: handle device allocation failure in btrfs_close_one_device() > btrfs: handle allocation failure in strdup > btrfs: handle error return of close_fs_devices() > btrfs: remove final BUG_ON() in close_fs_devices() > btrfs: change btrfs_fs_devices::seeing to bool > btrfs: change btrfs_fs_devices::rotating to bool I'll add the last 2 for now as they're obviously correct, the rest has some questions that I'd like to clarify before merging.
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 7c55169c0613..c58802c9c39c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1069,6 +1069,9 @@ static int btrfs_close_one_device(struct btrfs_device *device) new_device = btrfs_alloc_device(NULL, &device->devid, device->uuid); + btrfs_free_device(new_device); + pr_err("%s() INJECTING -ENOMEM\n", __func__); + new_device = ERR_PTR(-ENOMEM); if (IS_ERR(new_device)) return PTR_ERR(new_device);
This series attempts to remove the BUG_ON()s in btrfs_close_one_device(). Therefore some reorganization of btrfs_close_one_device() and close_fs_devices() was needed. Forthermore a new BUG_ON() had to be temporarily introduced but is removed again in the last patch of theis series. Although it is generally legal to return -ENOMEM on umount(2), the error handling up until close_ctree() as neither close_ctree() nor btrfs_put_super() would be able to handle the error. This series has passed fstests without any deviation from the baseline and also the new error handling was tested via error injection using this snippet: Changes to v1: Fixed the decremt of btrfs_fs_devices::seeding. In addition to this, I've added two patches changing btrfs_fs_devices::seeding and btrfs_fs_devices::rotating to bool, as they are in fact used as booleans. Johannes Thumshirn (7): btrfs: decrement number of open devices after closing the device not before btrfs: handle device allocation failure in btrfs_close_one_device() btrfs: handle allocation failure in strdup btrfs: handle error return of close_fs_devices() btrfs: remove final BUG_ON() in close_fs_devices() btrfs: change btrfs_fs_devices::seeing to bool btrfs: change btrfs_fs_devices::rotating to bool fs/btrfs/volumes.c | 81 ++++++++++++++++++++++++++++++++++++------------------ fs/btrfs/volumes.h | 4 +-- 2 files changed, 56 insertions(+), 29 deletions(-)