Skip to content

Commit 5fe6eaa

Browse files
chuckleveramschuma-ntap
chucklever
authored and
amschuma-ntap
committed
SUNRPC: Generalize the RPC buffer allocation API
xprtrdma needs to allocate the Call and Reply buffers separately. TBH, the reliance on using a single buffer for the pair of XDR buffers is transport implementation-specific. Transports that want to allocate separate Call and Reply buffers will ignore the "size" argument anyway. Don't bother passing it. The buf_alloc method can't return two pointers. Instead, make the method's return value an error code, and set the rq_buffer pointer in the method itself. This gives call_allocate an opportunity to terminate an RPC instead of looping forever when a permanent problem occurs. If a request is just bogus, or the transport is in a state where it can't allocate resources for any request, there needs to be a way to kill the RPC right there and not loop. This immediately fixes a rare problem in the backchannel send path, which loops if the server happens to send a CB request whose call+reply size is larger than a page (which it shouldn't do yet). One more issue: looks like xprt_inject_disconnect was incorrectly placed in the failure path in call_allocate. It needs to be in the success path, as it is for other call-sites. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
1 parent b9c5bc0 commit 5fe6eaa

File tree

7 files changed

+63
-37
lines changed

7 files changed

+63
-37
lines changed

‎include/linux/sunrpc/sched.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ struct rpc_task *rpc_wake_up_first(struct rpc_wait_queue *,
239239
void *);
240240
void rpc_wake_up_status(struct rpc_wait_queue *, int);
241241
void rpc_delay(struct rpc_task *, unsigned long);
242-
void * rpc_malloc(struct rpc_task *, size_t);
242+
int rpc_malloc(struct rpc_task *);
243243
void rpc_free(void *);
244244
int rpciod_up(void);
245245
void rpciod_down(void);

‎include/linux/sunrpc/xprt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ struct rpc_xprt_ops {
127127
void (*rpcbind)(struct rpc_task *task);
128128
void (*set_port)(struct rpc_xprt *xprt, unsigned short port);
129129
void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task);
130-
void * (*buf_alloc)(struct rpc_task *task, size_t size);
130+
int (*buf_alloc)(struct rpc_task *task);
131131
void (*buf_free)(void *buffer);
132132
int (*send_request)(struct rpc_task *task);
133133
void (*set_retrans_timeout)(struct rpc_task *task);

‎net/sunrpc/clnt.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,6 +1691,7 @@ call_allocate(struct rpc_task *task)
16911691
struct rpc_rqst *req = task->tk_rqstp;
16921692
struct rpc_xprt *xprt = req->rq_xprt;
16931693
struct rpc_procinfo *proc = task->tk_msg.rpc_proc;
1694+
int status;
16941695

16951696
dprint_status(task);
16961697

@@ -1716,11 +1717,14 @@ call_allocate(struct rpc_task *task)
17161717
req->rq_rcvsize = RPC_REPHDRSIZE + slack + proc->p_replen;
17171718
req->rq_rcvsize <<= 2;
17181719

1719-
req->rq_buffer = xprt->ops->buf_alloc(task,
1720-
req->rq_callsize + req->rq_rcvsize);
1721-
if (req->rq_buffer != NULL)
1722-
return;
1720+
status = xprt->ops->buf_alloc(task);
17231721
xprt_inject_disconnect(xprt);
1722+
if (status == 0)
1723+
return;
1724+
if (status != -ENOMEM) {
1725+
rpc_exit(task, status);
1726+
return;
1727+
}
17241728

17251729
dprintk("RPC: %5u rpc_buffer allocation failed\n", task->tk_pid);
17261730

‎net/sunrpc/sched.c

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -849,14 +849,17 @@ static void rpc_async_schedule(struct work_struct *work)
849849
}
850850

851851
/**
852-
* rpc_malloc - allocate an RPC buffer
853-
* @task: RPC task that will use this buffer
854-
* @size: requested byte size
852+
* rpc_malloc - allocate RPC buffer resources
853+
* @task: RPC task
854+
*
855+
* A single memory region is allocated, which is split between the
856+
* RPC call and RPC reply that this task is being used for. When
857+
* this RPC is retired, the memory is released by calling rpc_free.
855858
*
856859
* To prevent rpciod from hanging, this allocator never sleeps,
857-
* returning NULL and suppressing warning if the request cannot be serviced
858-
* immediately.
859-
* The caller can arrange to sleep in a way that is safe for rpciod.
860+
* returning -ENOMEM and suppressing warning if the request cannot
861+
* be serviced immediately. The caller can arrange to sleep in a
862+
* way that is safe for rpciod.
860863
*
861864
* Most requests are 'small' (under 2KiB) and can be serviced from a
862865
* mempool, ensuring that NFS reads and writes can always proceed,
@@ -865,8 +868,10 @@ static void rpc_async_schedule(struct work_struct *work)
865868
* In order to avoid memory starvation triggering more writebacks of
866869
* NFS requests, we avoid using GFP_KERNEL.
867870
*/
868-
void *rpc_malloc(struct rpc_task *task, size_t size)
871+
int rpc_malloc(struct rpc_task *task)
869872
{
873+
struct rpc_rqst *rqst = task->tk_rqstp;
874+
size_t size = rqst->rq_callsize + rqst->rq_rcvsize;
870875
struct rpc_buffer *buf;
871876
gfp_t gfp = GFP_NOIO | __GFP_NOWARN;
872877

@@ -880,12 +885,13 @@ void *rpc_malloc(struct rpc_task *task, size_t size)
880885
buf = kmalloc(size, gfp);
881886

882887
if (!buf)
883-
return NULL;
888+
return -ENOMEM;
884889

885890
buf->len = size;
886891
dprintk("RPC: %5u allocated buffer of size %zu at %p\n",
887892
task->tk_pid, size, buf);
888-
return &buf->data;
893+
rqst->rq_buffer = buf->data;
894+
return 0;
889895
}
890896
EXPORT_SYMBOL_GPL(rpc_malloc);
891897

‎net/sunrpc/xprtrdma/svc_rdma_backchannel.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,29 +159,30 @@ static int svc_rdma_bc_sendto(struct svcxprt_rdma *rdma,
159159
/* Server-side transport endpoint wants a whole page for its send
160160
* buffer. The client RPC code constructs the RPC header in this
161161
* buffer before it invokes ->send_request.
162-
*
163-
* Returns NULL if there was a temporary allocation failure.
164162
*/
165-
static void *
166-
xprt_rdma_bc_allocate(struct rpc_task *task, size_t size)
163+
static int
164+
xprt_rdma_bc_allocate(struct rpc_task *task)
167165
{
168166
struct rpc_rqst *rqst = task->tk_rqstp;
169167
struct svc_xprt *sxprt = rqst->rq_xprt->bc_xprt;
168+
size_t size = rqst->rq_callsize;
170169
struct svcxprt_rdma *rdma;
171170
struct page *page;
172171

173172
rdma = container_of(sxprt, struct svcxprt_rdma, sc_xprt);
174173

175-
/* Prevent an infinite loop: try to make this case work */
176-
if (size > PAGE_SIZE)
174+
if (size > PAGE_SIZE) {
177175
WARN_ONCE(1, "svcrdma: large bc buffer request (size %zu)\n",
178176
size);
177+
return -EINVAL;
178+
}
179179

180180
page = alloc_page(RPCRDMA_DEF_GFP);
181181
if (!page)
182-
return NULL;
182+
return -ENOMEM;
183183

184-
return page_address(page);
184+
rqst->rq_buffer = page_address(page);
185+
return 0;
185186
}
186187

187188
static void

‎net/sunrpc/xprtrdma/transport.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,15 @@ xprt_rdma_connect(struct rpc_xprt *xprt, struct rpc_task *task)
477477
}
478478
}
479479

480-
/*
480+
/**
481+
* xprt_rdma_allocate - allocate transport resources for an RPC
482+
* @task: RPC task
483+
*
484+
* Return values:
485+
* 0: Success; rq_buffer points to RPC buffer to use
486+
* ENOMEM: Out of memory, call again later
487+
* EIO: A permanent error occurred, do not retry
488+
*
481489
* The RDMA allocate/free functions need the task structure as a place
482490
* to hide the struct rpcrdma_req, which is necessary for the actual send/recv
483491
* sequence.
@@ -486,19 +494,20 @@ xprt_rdma_connect(struct rpc_xprt *xprt, struct rpc_task *task)
486494
* (rq_send_buf and rq_rcv_buf are both part of a single contiguous buffer).
487495
* We may register rq_rcv_buf when using reply chunks.
488496
*/
489-
static void *
490-
xprt_rdma_allocate(struct rpc_task *task, size_t size)
497+
static int
498+
xprt_rdma_allocate(struct rpc_task *task)
491499
{
492-
struct rpc_xprt *xprt = task->tk_rqstp->rq_xprt;
493-
struct rpcrdma_xprt *r_xprt = rpcx_to_rdmax(xprt);
500+
struct rpc_rqst *rqst = task->tk_rqstp;
501+
size_t size = rqst->rq_callsize + rqst->rq_rcvsize;
502+
struct rpcrdma_xprt *r_xprt = rpcx_to_rdmax(rqst->rq_xprt);
494503
struct rpcrdma_regbuf *rb;
495504
struct rpcrdma_req *req;
496505
size_t min_size;
497506
gfp_t flags;
498507

499508
req = rpcrdma_buffer_get(&r_xprt->rx_buf);
500509
if (req == NULL)
501-
return NULL;
510+
return -ENOMEM;
502511

503512
flags = RPCRDMA_DEF_GFP;
504513
if (RPC_IS_SWAPPER(task))
@@ -515,7 +524,8 @@ xprt_rdma_allocate(struct rpc_task *task, size_t size)
515524
dprintk("RPC: %s: size %zd, request 0x%p\n", __func__, size, req);
516525
req->rl_connect_cookie = 0; /* our reserved value */
517526
req->rl_task = task;
518-
return req->rl_sendbuf->rg_base;
527+
rqst->rq_buffer = req->rl_sendbuf->rg_base;
528+
return 0;
519529

520530
out_rdmabuf:
521531
min_size = r_xprt->rx_data.inline_wsize;
@@ -558,7 +568,7 @@ xprt_rdma_allocate(struct rpc_task *task, size_t size)
558568

559569
out_fail:
560570
rpcrdma_buffer_put(req);
561-
return NULL;
571+
return -ENOMEM;
562572
}
563573

564574
/*

‎net/sunrpc/xprtsock.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2533,23 +2533,28 @@ static void xs_tcp_print_stats(struct rpc_xprt *xprt, struct seq_file *seq)
25332533
* we allocate pages instead doing a kmalloc like rpc_malloc is because we want
25342534
* to use the server side send routines.
25352535
*/
2536-
static void *bc_malloc(struct rpc_task *task, size_t size)
2536+
static int bc_malloc(struct rpc_task *task)
25372537
{
2538+
struct rpc_rqst *rqst = task->tk_rqstp;
2539+
size_t size = rqst->rq_callsize;
25382540
struct page *page;
25392541
struct rpc_buffer *buf;
25402542

2541-
WARN_ON_ONCE(size > PAGE_SIZE - sizeof(struct rpc_buffer));
2542-
if (size > PAGE_SIZE - sizeof(struct rpc_buffer))
2543-
return NULL;
2543+
if (size > PAGE_SIZE - sizeof(struct rpc_buffer)) {
2544+
WARN_ONCE(1, "xprtsock: large bc buffer request (size %zu)\n",
2545+
size);
2546+
return -EINVAL;
2547+
}
25442548

25452549
page = alloc_page(GFP_KERNEL);
25462550
if (!page)
2547-
return NULL;
2551+
return -ENOMEM;
25482552

25492553
buf = page_address(page);
25502554
buf->len = PAGE_SIZE;
25512555

2552-
return buf->data;
2556+
rqst->rq_buffer = buf->data;
2557+
return 0;
25532558
}
25542559

25552560
/*

0 commit comments

Comments
 (0)