Re: [PATCH RFC] SUNRPC: Avoid digging into the ATOMIC pool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Mar 4, 2019, at 11:02 AM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
> 
> On Mon, 2019-03-04 at 10:32 -0500, Chuck Lever wrote:
>> Page allocation requests made when the SPARSE_PAGES flag is set are
>> allowed to fail, and are not critical. No need to spend a rare
>> resource.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> net/sunrpc/socklib.c           |    2 +-
>> net/sunrpc/xprtrdma/rpc_rdma.c |    2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
>> index 7e55cfc..9faea12 100644
>> --- a/net/sunrpc/socklib.c
>> +++ b/net/sunrpc/socklib.c
>> @@ -106,7 +106,7 @@ static size_t xdr_skb_read_and_csum_bits(struct
>> xdr_skb_reader *desc, void *to,
>> 		/* ACL likes to be lazy in allocating pages - ACLs
>> 		 * are small by default but can get huge. */
>> 		if ((xdr->flags & XDRBUF_SPARSE_PAGES) && *ppage ==
>> NULL) {
>> -			*ppage = alloc_page(GFP_ATOMIC);
>> +			*ppage = alloc_page(GFP_NOWAIT | __GFP_NOWARN);
>> 			if (unlikely(*ppage == NULL)) {
>> 				if (copied == 0)
>> 					copied = -ENOMEM;
> 
> Hmm... Can't we make this GFP_KERNEL? I can't see that we're holding
> any locks here.

We're holding the transport send lock. Since this is an order 0 allocation
it's unlikely to fail even with a less aggressive request like NOWAIT.


> It does look like svc_udp_recvfrom() is wrapping the
> call to csum_partial_copy_to_xdr() in a local_bh_disable()/enable()
> pair, but is that actually needed? If so, for what?
> 
>> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c
>> b/net/sunrpc/xprtrdma/rpc_rdma.c
>> index 6c1fb27..b759b16 100644
>> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
>> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
>> @@ -238,7 +238,7 @@ static bool rpcrdma_results_inline(struct
>> rpcrdma_xprt *r_xprt,
>> 		 */
>> 		if (unlikely(xdrbuf->flags & XDRBUF_SPARSE_PAGES)) {
>> 			if (!*ppages)
>> -				*ppages = alloc_page(GFP_ATOMIC);
>> +				*ppages = alloc_page(GFP_NOWAIT |
>> __GFP_NOWARN);
>> 			if (!*ppages)
>> 				return -ENOBUFS;
>> 		}
>> 
> 
> Cheers
>  Trond
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx

--
Chuck Lever







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux