Re: [PATCH v2] Btrfs: fix deadlock with memory reclaim during scrub

[Nikolay Borisov <nborisov@suse.com>]

On 23.11.18 г. 18:05 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When a transaction commit starts, it attempts to pause scrub and it blocks
> until the scrub is paused. So while the transaction is blocked waiting for
> scrub to pause, we can not do memory allocation with GFP_KERNEL while scrub
> is running, we must use GFP_NOS to avoid deadlock with reclaim. Checking
> for pause requests is done early in the while loop of scrub_stripe(), and
> later in the loop, scrub_extent() is called, which in turns calls
> scrub_pages(), which does memory allocations using GFP_KERNEL. So use
> GFP_NOFS for the memory allocations because at any time a scrub pause
> request can happen from another task that started to commit a transaction.
> 
> Fixes: 58c4e173847a ("btrfs: scrub: use GFP_KERNEL on the submission path")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Make using GFP_NOFS unconditionial. Previous version was racy, as pausing
> requests migth happen just after we checked for them.
> 
>  fs/btrfs/scrub.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 3be1456b5116..0630ea0881bc 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2205,7 +2205,13 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len,
>  	struct scrub_block *sblock;
>  	int index;
>  
> -	sblock = kzalloc(sizeof(*sblock), GFP_KERNEL);
> +	/*
> +	 * In order to avoid deadlock with reclaim when there is a transaction
> +	 * trying to pause scrub, use GFP_NOFS. The pausing request is done when
> +	 * the transaction commit starts, and it blocks the transaction until
> +	 * scrub is paused (done at specific points at scrub_stripe()).
> +	 */
> +	sblock = kzalloc(sizeof(*sblock), GFP_NOFS);

Newer code shouldn't use GFP_NOFS, rather leave GFP_KERNEL as is and
instead use the memaloc_nofs_save/memalloc_nofs_restore. For background
information refer to: Documentation/core-api/gfp_mask-from-fs-io.rst

>  	if (!sblock) {
>  		spin_lock(&sctx->stat_lock);
>  		sctx->stat.malloc_errors++;
> @@ -2223,7 +2229,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len,
>  		struct scrub_page *spage;
>  		u64 l = min_t(u64, len, PAGE_SIZE);
>  
> -		spage = kzalloc(sizeof(*spage), GFP_KERNEL);
> +		spage = kzalloc(sizeof(*spage), GFP_NOFS);
>  		if (!spage) {
>  leave_nomem:
>  			spin_lock(&sctx->stat_lock);
> @@ -2250,7 +2256,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len,
>  			spage->have_csum = 0;
>  		}
>  		sblock->page_count++;
> -		spage->page = alloc_page(GFP_KERNEL);
> +		spage->page = alloc_page(GFP_NOFS);
>  		if (!spage->page)
>  			goto leave_nomem;
>  		len -= l;
>