On Wed, Nov 13, 2019 at 11:27:26AM +0100, Johannes Thumshirn wrote:
> Now that the preparation work is done, remove the temporary BUG_ON() in
> close_fs_devices() and return an error instead.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
>
> ---
> Changes to v1:
> - btrfs_fs_devices::seeding is a 'boolean' flags and no counter, don't
> decrement it (Qu)
> ---
> 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 be1fd935edf7..25e4608e20f1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1128,7 +1128,11 @@ static int close_fs_devices(struct btrfs_fs_devices *fs_devices)
> mutex_lock(&fs_devices->device_list_mutex);
> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
> ret = btrfs_close_one_device(device);
> - BUG_ON(ret); /* -ENOMEM */
> + if (ret) {
> + mutex_unlock(&fs_devices->device_list_mutex);
> + return ret;
This can fail in the middle of the loop thus leaving some devices in the
list and keeping open_devices half-changed.
Not all callers of close_fs_devices handle the errors so this can break
invariants, where eg. fs_devices->opened is expected to be 0 after the
function call, similar for ->seeding or ->rw_devices.
> + }
> + fs_devices->opened--;
> }
> mutex_unlock(&fs_devices->device_list_mutex);
>
> --
> 2.16.4
