On Mon, 2019-03-04 at 11:05 -0500, Chuck Lever wrote:
> > 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.
We should only be holding the receive mutex. That prevents others from
closing the socket (or starting a second receive worker) but that's
all.
OTOH, the only thing using this mechanism on UDP should be the NFSv2/v3
ACL receive code, so I'm not really sure anyone cares.
> > 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
>
>
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
