On Tue, May 08, 2018 at 12:34:48PM -0400, Chuck Lever wrote: > > > > On May 8, 2018, at 12:15 PM, bfie...@fieldses.org wrote: > > > > On Tue, Apr 17, 2018 at 09:54:36PM +0000, Trond Myklebust wrote: > >> Yes, and we can probably convert it, and the other GFP_ATOMIC > >> allocations in the rpcbind client to use GFP_NOFS in order to improve > >> reliability. > > > > Chuck, I think the GFP_ATOMIC is unnecessary here as well? > > > > --b. > > > > diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c > > index e8adad33d0bb..de90c6c90cde 100644 > > --- a/net/sunrpc/xprtrdma/rpc_rdma.c > > +++ b/net/sunrpc/xprtrdma/rpc_rdma.c > > @@ -228,7 +228,7 @@ rpcrdma_convert_iovs(struct rpcrdma_xprt *r_xprt, > > struct xdr_buf *xdrbuf, > > /* XXX: Certain upper layer operations do > > * not provide receive buffer pages. > > */ > > - *ppages = alloc_page(GFP_ATOMIC); > > + *ppages = alloc_page(GFP_NOFS); > > if (!*ppages) > > return -EAGAIN; > > } > > This code can't sleep, as I understand it. Caller is holding > the transport write lock. This logic was copied from > xdr_partial_copy_from_skb, which uses GFP_ATOMIC.
OK. > Recall that this is here because of GETACL. As I've stated in > the past, the correct solution is to ensure that these pages > are provided in every case by the upper layer, making this > alloc_page call site unnecessary. Got it. --b.
