Re: [PATCH 2/2] btrfs: stop doing GFP_KERNEL memory allocations in the ref verify tool

[David Sterba <dsterba@suse.cz>]

On Tue, Jul 20, 2021 at 04:05:23PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> In commit 351cbf6e4410e7 ("btrfs: use nofs allocations for running delayed
> items") we wrapped all btree updates when running delayed items with
> memalloc_nofs_save() and memalloc_nofs_restore(), due to a lock inversion
> detected by lockdep involving reclaim and the mutex of delayed nodes.
> 
> The problem is because the ref verify tool does some memory allocations
> with GFP_KERNEL, which can trigger reclaim and reclaim can trigger inode
> eviction, which requires locking the mutex of an inode's delayed node.
> On the other hand the ref verify tool is called when allocating metadata
> extents as part of operations that modify a btree, which is a problem when
> running delayed nodes, where we do btree updates while holding the mutex
> of a delayed node. This is what caused the lockdep warning.
> 
> Instead of wrapping every btree update when running delayed nodes, change
> the ref verify tool to never do GFP_KERNEL allocations, because:
> 
> 1) We get less repeated code, which at the moment does not even have a
>    comment mentioning why we need to setup the NOFS context, which is a
>    recommended good practice as mentioned at
>    Documentation/core-api/gfp_mask-from-fs-io.rst
> 
> 2) The ref verify tool is something meant only for debugging and not
>    something that should be enabled on non-debug / non-development
>    kernels;
> 
> 3) We may have yet more places outside delayed-inode.c where we have
>    similar problem: doing btree updates while holding some lock and
>    then having the GFP_KERNEL memory allocations, from the ref verify
>    tool, trigger reclaim and trying again to acquire the same lock
>    through the reclaim path.
>    Or we could get more such cases in the future, therefore this change
>    prevents getting into similar cases when using the ref verify tool.

That all sounds reasonable to me regarding the GFP flags use.