Re: [PATCH] btrfs: fix lockdep warning chunk_mutex vs device_list_mutex

[Anand Jain <anand.jain@oracle.com>]

On 16/5/20 11:43 am, Anand Jain wrote:
> On 16/5/20 1:40 am, David Sterba wrote:
>> On Thu, May 14, 2020 at 03:46:59AM +0800, Anand Jain wrote:
>>> A full list of tests just started.
>>>
>>>   fs/btrfs/volumes.c | 8 +++++---
>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 60ab41c12e50..ebc8565d0f73 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -984,7 +984,6 @@ static struct btrfs_fs_devices 
>>> *clone_fs_devices(struct btrfs_fs_devices *orig)
>>>       if (IS_ERR(fs_devices))
>>>           return fs_devices;
>>
>> So now here's the device_list_mutex taken by a caller but inside
>> clone_fs_devices there's
>>
>>     fs_devices = alloc_fs_devices(orig->fsid, NULL);
>>
>> just before this line and it does a GFP_KERNEL allocation.
> 
> Oh right the allocations. Its not just about the other locks
> as I thought before.
> 
> There are two ways to fix.
>    Use GFP_NOFS
>     I am not yet sure if it not possible. There were some previous
>     work on the GFP flags. I need to review them. or,
>    Move the allocation outside the locks.
> 
>   Looking into both of these choices.
> 


Nack. On this patch.

In general GFP_KERNEL is preferred over GFP_NOFS. For example.

--------
6165572c btrfs: use GFP_KERNEL in btrfs_init_dev_replace_tgtdev
cc8385b5 btrfs: preallocate radix tree node for readahead
78f2c9e6 btrfs: device add and remove: use GFP_KERNEL
--------

And there are quite a lot of GFP_KERNEL allocation along the
path leading to clone_fs_devices().


Thanks, Anand

> Thanks, Anand
> 
>> This could
>> deadlock through the allocator trying to flush data and then superblock
>> write locking the device_list_mutex again.
>>
>>> -    mutex_lock(&orig->device_list_mutex);
>>>       fs_devices->total_devices = orig->total_devices;
>>>       list_for_each_entry(orig_dev, &orig->devices, dev_list) {
>