Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     trondmy@kernel.org
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH] NFS: Fix memory allocation in rpc_malloc()
In-reply-to: <20220315162052.570677-1-trondmy@kernel.org>
References: <20220315162052.570677-1-trondmy@kernel.org>
Date:   Wed, 16 Mar 2022 12:53:04 +1100
Message-id: <164739558440.9764.6759307849718646101@noble.neil.brown.name>
Precedence: bulk

On Wed, 16 Mar 2022, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> When allocating memory, it should be safe to always use GFP_KERNEL,
> since both swap tasks and asynchronous tasks will regulate the
> allocation mode through the struct task flags.

'struct task_struct' flags can only enforce NOFS, NOIO, or MEMALLOC.
They cannot prevent waiting altogether.
We need rpciod task to not block waiting for memory.  If they all do,
then there will be no thread free to handle the replies to WRITE which
would allow swapped-out memory to be freed.

As the very least the rescuer thread mustn't block, so the use of
GFP_NOWAIT could depend on current_is_workqueue_rescuer().

Was there some problem you saw caused by the use of GFP_NOWAIT ??

Thanks,
NeilBrown

> 
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  net/sunrpc/sched.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index 7c8f87ebdbc0..c62fcacf7366 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -1030,16 +1030,12 @@ int rpc_malloc(struct rpc_task *task)
>  	struct rpc_rqst *rqst = task->tk_rqstp;
>  	size_t size = rqst->rq_callsize + rqst->rq_rcvsize;
>  	struct rpc_buffer *buf;
> -	gfp_t gfp = GFP_KERNEL;
> -
> -	if (RPC_IS_ASYNC(task))
> -		gfp = GFP_NOWAIT | __GFP_NOWARN;
>  
>  	size += sizeof(struct rpc_buffer);
>  	if (size <= RPC_BUFFER_MAXSIZE)
> -		buf = mempool_alloc(rpc_buffer_mempool, gfp);
> +		buf = mempool_alloc(rpc_buffer_mempool, GFP_KERNEL);
>  	else
> -		buf = kmalloc(size, gfp);
> +		buf = kmalloc(size, GFP_KERNEL);
>  
>  	if (!buf)
>  		return -ENOMEM;
> -- 
> 2.35.1
> 
>