| Message ID | 160625754220.280431.690992380938118353.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] SUNRPC: Remove XDRBUF_SPARSE_PAGES flag in gss_proxy upcall | expand |
On Tue, Nov 24, 2020 at 7:04 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > Commit 9dfd87da1aeb ("rpc: fix huge kmalloc's in gss-proxy") added > gssp_alloc_receive_pages() to fully allocate the receive buffer > for gss_proxy upcalls. > > However, later, 431f6eb3570f ("SUNRPC: Add a label for RPC calls > that require allocation on receive") sets the XDRBUF_SPARSE_PAGES > flag for this receive buffer anyway. That doesn't appear to have > been necessary, since gssp_alloc_receive_pages() still exists. But the gssp_alloc_receive_pages() only allocates the array of page pointers not the actual pages, so I believe the flag is still needed to have those pages allocated by something? What is allocating those pages if not the SPARSE_PAGES method, what am I missing? > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c > index 2ff7b7083eba..44838f6ea25e 100644 > --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c > +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c > @@ -771,7 +771,6 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req, > xdr_inline_pages(&req->rq_rcv_buf, > PAGE_SIZE/2 /* pretty arbitrary */, > arg->pages, 0 /* page base */, arg->npages * PAGE_SIZE); > - req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES; > done: > if (err) > dprintk("RPC: gssx_enc_accept_sec_context: %d\n", err); > >
> On Nov 30, 2020, at 2:33 PM, Olga Kornievskaia <aglo@umich.edu> wrote: > > On Tue, Nov 24, 2020 at 7:04 PM Chuck Lever <chuck.lever@oracle.com> wrote: >> >> Commit 9dfd87da1aeb ("rpc: fix huge kmalloc's in gss-proxy") added >> gssp_alloc_receive_pages() to fully allocate the receive buffer >> for gss_proxy upcalls. >> >> However, later, 431f6eb3570f ("SUNRPC: Add a label for RPC calls >> that require allocation on receive") sets the XDRBUF_SPARSE_PAGES >> flag for this receive buffer anyway. That doesn't appear to have >> been necessary, since gssp_alloc_receive_pages() still exists. > > But the gssp_alloc_receive_pages() only allocates the array of page > pointers not the actual pages, so I believe the flag is still needed > to have those pages allocated by something? What is allocating those > pages if not the SPARSE_PAGES method, what am I missing? Ugh <face palm> gssp_alloc_receive_pages() will have to allocate those pages. Again, I don't see any reason to defer page allocation from a context in which GFP_KERNEL is allowed to one where it isn't, and where we know exactly how many pages are needed. I'll respin. Good catch! >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c >> index 2ff7b7083eba..44838f6ea25e 100644 >> --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c >> +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c >> @@ -771,7 +771,6 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req, >> xdr_inline_pages(&req->rq_rcv_buf, >> PAGE_SIZE/2 /* pretty arbitrary */, >> arg->pages, 0 /* page base */, arg->npages * PAGE_SIZE); >> - req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES; >> done: >> if (err) >> dprintk("RPC: gssx_enc_accept_sec_context: %d\n", err); >> >> -- Chuck Lever
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c index 2ff7b7083eba..44838f6ea25e 100644 --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c @@ -771,7 +771,6 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req, xdr_inline_pages(&req->rq_rcv_buf, PAGE_SIZE/2 /* pretty arbitrary */, arg->pages, 0 /* page base */, arg->npages * PAGE_SIZE); - req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES; done: if (err) dprintk("RPC: gssx_enc_accept_sec_context: %d\n", err);
Commit 9dfd87da1aeb ("rpc: fix huge kmalloc's in gss-proxy") added gssp_alloc_receive_pages() to fully allocate the receive buffer for gss_proxy upcalls. However, later, 431f6eb3570f ("SUNRPC: Add a label for RPC calls that require allocation on receive") sets the XDRBUF_SPARSE_PAGES flag for this receive buffer anyway. That doesn't appear to have been necessary, since gssp_alloc_receive_pages() still exists. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- net/sunrpc/auth_gss/gss_rpc_xdr.c | 1 - 1 file changed, 1 deletion(-)