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 <[email protected]>
---
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 Tue, Nov 24, 2020 at 7:04 PM Chuck Lever <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
> On Tue, Nov 24, 2020 at 7:04 PM Chuck Lever <[email protected]> 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 <[email protected]>
>> ---
>> 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