linux-kernel - Re: [PATCH v4 1/3] btrfs: add might_sleep() to some places in update_qgroup_limit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3918175e-dddd-2a55-32c4-c07de78ff4cb@gmx.com>
Date:   Wed, 16 Nov 2022 16:43:50 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     ChenXiaoSong <chenxiaosong2@...wei.com>, clm@...com,
        josef@...icpanda.com, dsterba@...e.com
Cc:     linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        zhangxiaoxu5@...wei.com, yanaijie@...wei.com, wqu@...e.com
Subject: Re: [PATCH v4 1/3] btrfs: add might_sleep() to some places in
 update_qgroup_limit_item()



On 2022/11/16 16:09, ChenXiaoSong wrote:
> 在 2022/11/16 6:48, Qu Wenruo 写道:
>> Looks good.
>>
>> We may want to add more in other locations, but this is really a good 
>> start.
>>
>> Reviewed-by: Qu Wenruo <wqu@...e.com>
>>
>> Thanks,
>> Qu
> 
> If I just add might_sleep() in btrfs_alloc_path() and 
> btrfs_search_slot(), is it reasonable?

Adding it to btrfs_search_slot() is definitely correct.

But why for btrfs_alloc_path()? Wouldn't kmem_cache_zalloc() itself 
already do the might_sleep_if() somewhere?

I just looked the call chain, and indeed it is doing the check already:

btrfs_alloc_path()
|- kmem_cache_zalloc()
    |- kmem_cache_alloc()
       |- __kmem_cache_alloc_lru()
          |- slab_alloc()
             |- slab_alloc_node()
                |- slab_pre_alloc_hook()
                   |- might_alloc()
                      |- might_sleep_if()

Thanks,
Qu

> 
> Or just add might_sleep() to one place in update_qgroup_limit_item() ?