On 13/11/2019 16:02, David Sterba wrote:
> 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.
Please see my answer to "btrfs: handle device allocation failure in
btrfs_close_one_device()" on this topic.
--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@xxxxxxx +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
