diff options
author | Chuck Lever <chuck.lever@oracle.com> | 2007-03-29 16:47:53 -0400 |
---|---|---|
committer | Trond Myklebust <Trond.Myklebust@netapp.com> | 2007-05-01 01:17:10 -0400 |
commit | 2bea90d43a050bbc4021d44e59beb34f384438db (patch) | |
tree | 2dd3f15bd9df537166a82777b0c95243b90b17e1 /net/sunrpc | |
parent | 511d2e8855a065c8251d0c140ebc353854f1929e (diff) |
SUNRPC: RPC buffer size estimates are too large
The RPC buffer size estimation logic in net/sunrpc/clnt.c always
significantly overestimates the requirements for the buffer size.
A little instrumentation demonstrated that in fact rpc_malloc was never
allocating the buffer from the mempool, but almost always called kmalloc.
To compute the size of the RPC buffer more precisely, split p_bufsiz into
two fields; one for the argument size, and one for the result size.
Then, compute the sum of the exact call and reply header sizes, and split
the RPC buffer precisely between the two. That should keep almost all RPC
buffers within the 2KiB buffer mempool limit.
And, we can finally be rid of RPC_SLACK_SPACE!
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Diffstat (limited to 'net/sunrpc')
-rw-r--r-- | net/sunrpc/clnt.c | 62 | ||||
-rw-r--r-- | net/sunrpc/pmap_clnt.c | 9 | ||||
-rw-r--r-- | net/sunrpc/xprt.c | 1 |
3 files changed, 44 insertions, 28 deletions
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 396cdbe249d1..12487aafaab5 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c | |||
@@ -36,8 +36,6 @@ | |||
36 | #include <linux/sunrpc/metrics.h> | 36 | #include <linux/sunrpc/metrics.h> |
37 | 37 | ||
38 | 38 | ||
39 | #define RPC_SLACK_SPACE (1024) /* total overkill */ | ||
40 | |||
41 | #ifdef RPC_DEBUG | 39 | #ifdef RPC_DEBUG |
42 | # define RPCDBG_FACILITY RPCDBG_CALL | 40 | # define RPCDBG_FACILITY RPCDBG_CALL |
43 | #endif | 41 | #endif |
@@ -747,21 +745,37 @@ call_reserveresult(struct rpc_task *task) | |||
747 | static void | 745 | static void |
748 | call_allocate(struct rpc_task *task) | 746 | call_allocate(struct rpc_task *task) |
749 | { | 747 | { |
748 | unsigned int slack = task->tk_auth->au_cslack; | ||
750 | struct rpc_rqst *req = task->tk_rqstp; | 749 | struct rpc_rqst *req = task->tk_rqstp; |
751 | struct rpc_xprt *xprt = task->tk_xprt; | 750 | struct rpc_xprt *xprt = task->tk_xprt; |
752 | unsigned int bufsiz; | 751 | struct rpc_procinfo *proc = task->tk_msg.rpc_proc; |
753 | 752 | ||
754 | dprint_status(task); | 753 | dprint_status(task); |
755 | 754 | ||
755 | task->tk_status = 0; | ||
756 | task->tk_action = call_bind; | 756 | task->tk_action = call_bind; |
757 | |||
757 | if (req->rq_buffer) | 758 | if (req->rq_buffer) |
758 | return; | 759 | return; |
759 | 760 | ||
760 | /* FIXME: compute buffer requirements more exactly using | 761 | if (proc->p_proc != 0) { |
761 | * auth->au_wslack */ | 762 | BUG_ON(proc->p_arglen == 0); |
762 | bufsiz = task->tk_msg.rpc_proc->p_bufsiz + RPC_SLACK_SPACE; | 763 | if (proc->p_decode != NULL) |
764 | BUG_ON(proc->p_replen == 0); | ||
765 | } | ||
763 | 766 | ||
764 | if (xprt->ops->buf_alloc(task, bufsiz << 1) != NULL) | 767 | /* |
768 | * Calculate the size (in quads) of the RPC call | ||
769 | * and reply headers, and convert both values | ||
770 | * to byte sizes. | ||
771 | */ | ||
772 | req->rq_callsize = RPC_CALLHDRSIZE + (slack << 1) + proc->p_arglen; | ||
773 | req->rq_callsize <<= 2; | ||
774 | req->rq_rcvsize = RPC_REPHDRSIZE + slack + proc->p_replen; | ||
775 | req->rq_rcvsize <<= 2; | ||
776 | |||
777 | xprt->ops->buf_alloc(task, req->rq_callsize + req->rq_rcvsize); | ||
778 | if (req->rq_buffer != NULL) | ||
765 | return; | 779 | return; |
766 | 780 | ||
767 | dprintk("RPC: %5u rpc_buffer allocation failed\n", task->tk_pid); | 781 | dprintk("RPC: %5u rpc_buffer allocation failed\n", task->tk_pid); |
@@ -788,6 +802,17 @@ rpc_task_force_reencode(struct rpc_task *task) | |||
788 | task->tk_rqstp->rq_snd_buf.len = 0; | 802 | task->tk_rqstp->rq_snd_buf.len = 0; |
789 | } | 803 | } |
790 | 804 | ||
805 | static inline void | ||
806 | rpc_xdr_buf_init(struct xdr_buf *buf, void *start, size_t len) | ||
807 | { | ||
808 | buf->head[0].iov_base = start; | ||
809 | buf->head[0].iov_len = len; | ||
810 | buf->tail[0].iov_len = 0; | ||
811 | buf->page_len = 0; | ||
812 | buf->len = 0; | ||
813 | buf->buflen = len; | ||
814 | } | ||
815 | |||
791 | /* | 816 | /* |
792 | * 3. Encode arguments of an RPC call | 817 | * 3. Encode arguments of an RPC call |
793 | */ | 818 | */ |
@@ -795,28 +820,17 @@ static void | |||
795 | call_encode(struct rpc_task *task) | 820 | call_encode(struct rpc_task *task) |
796 | { | 821 | { |
797 | struct rpc_rqst *req = task->tk_rqstp; | 822 | struct rpc_rqst *req = task->tk_rqstp; |
798 | struct xdr_buf *sndbuf = &req->rq_snd_buf; | ||
799 | struct xdr_buf *rcvbuf = &req->rq_rcv_buf; | ||
800 | unsigned int bufsiz; | ||
801 | kxdrproc_t encode; | 823 | kxdrproc_t encode; |
802 | __be32 *p; | 824 | __be32 *p; |
803 | 825 | ||
804 | dprint_status(task); | 826 | dprint_status(task); |
805 | 827 | ||
806 | /* Default buffer setup */ | 828 | rpc_xdr_buf_init(&req->rq_snd_buf, |
807 | bufsiz = req->rq_bufsize >> 1; | 829 | req->rq_buffer, |
808 | sndbuf->head[0].iov_base = (void *)req->rq_buffer; | 830 | req->rq_callsize); |
809 | sndbuf->head[0].iov_len = bufsiz; | 831 | rpc_xdr_buf_init(&req->rq_rcv_buf, |
810 | sndbuf->tail[0].iov_len = 0; | 832 | (char *)req->rq_buffer + req->rq_callsize, |
811 | sndbuf->page_len = 0; | 833 | req->rq_rcvsize); |
812 | sndbuf->len = 0; | ||
813 | sndbuf->buflen = bufsiz; | ||
814 | rcvbuf->head[0].iov_base = (void *)((char *)req->rq_buffer + bufsiz); | ||
815 | rcvbuf->head[0].iov_len = bufsiz; | ||
816 | rcvbuf->tail[0].iov_len = 0; | ||
817 | rcvbuf->page_len = 0; | ||
818 | rcvbuf->len = 0; | ||
819 | rcvbuf->buflen = bufsiz; | ||
820 | 834 | ||
821 | /* Encode header and provided arguments */ | 835 | /* Encode header and provided arguments */ |
822 | encode = task->tk_msg.rpc_proc->p_encode; | 836 | encode = task->tk_msg.rpc_proc->p_encode; |
diff --git a/net/sunrpc/pmap_clnt.c b/net/sunrpc/pmap_clnt.c index d9f765344589..c45fc4c99513 100644 --- a/net/sunrpc/pmap_clnt.c +++ b/net/sunrpc/pmap_clnt.c | |||
@@ -335,7 +335,8 @@ static struct rpc_procinfo pmap_procedures[] = { | |||
335 | .p_proc = PMAP_SET, | 335 | .p_proc = PMAP_SET, |
336 | .p_encode = (kxdrproc_t) xdr_encode_mapping, | 336 | .p_encode = (kxdrproc_t) xdr_encode_mapping, |
337 | .p_decode = (kxdrproc_t) xdr_decode_bool, | 337 | .p_decode = (kxdrproc_t) xdr_decode_bool, |
338 | .p_bufsiz = 4, | 338 | .p_arglen = 4, |
339 | .p_replen = 1, | ||
339 | .p_count = 1, | 340 | .p_count = 1, |
340 | .p_statidx = PMAP_SET, | 341 | .p_statidx = PMAP_SET, |
341 | .p_name = "SET", | 342 | .p_name = "SET", |
@@ -344,7 +345,8 @@ static struct rpc_procinfo pmap_procedures[] = { | |||
344 | .p_proc = PMAP_UNSET, | 345 | .p_proc = PMAP_UNSET, |
345 | .p_encode = (kxdrproc_t) xdr_encode_mapping, | 346 | .p_encode = (kxdrproc_t) xdr_encode_mapping, |
346 | .p_decode = (kxdrproc_t) xdr_decode_bool, | 347 | .p_decode = (kxdrproc_t) xdr_decode_bool, |
347 | .p_bufsiz = 4, | 348 | .p_arglen = 4, |
349 | .p_replen = 1, | ||
348 | .p_count = 1, | 350 | .p_count = 1, |
349 | .p_statidx = PMAP_UNSET, | 351 | .p_statidx = PMAP_UNSET, |
350 | .p_name = "UNSET", | 352 | .p_name = "UNSET", |
@@ -353,7 +355,8 @@ static struct rpc_procinfo pmap_procedures[] = { | |||
353 | .p_proc = PMAP_GETPORT, | 355 | .p_proc = PMAP_GETPORT, |
354 | .p_encode = (kxdrproc_t) xdr_encode_mapping, | 356 | .p_encode = (kxdrproc_t) xdr_encode_mapping, |
355 | .p_decode = (kxdrproc_t) xdr_decode_port, | 357 | .p_decode = (kxdrproc_t) xdr_decode_port, |
356 | .p_bufsiz = 4, | 358 | .p_arglen = 4, |
359 | .p_replen = 1, | ||
357 | .p_count = 1, | 360 | .p_count = 1, |
358 | .p_statidx = PMAP_GETPORT, | 361 | .p_statidx = PMAP_GETPORT, |
359 | .p_name = "GETPORT", | 362 | .p_name = "GETPORT", |
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index 456a14510308..432ee92cf262 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c | |||
@@ -823,7 +823,6 @@ static void xprt_request_init(struct rpc_task *task, struct rpc_xprt *xprt) | |||
823 | req->rq_task = task; | 823 | req->rq_task = task; |
824 | req->rq_xprt = xprt; | 824 | req->rq_xprt = xprt; |
825 | req->rq_buffer = NULL; | 825 | req->rq_buffer = NULL; |
826 | req->rq_bufsize = 0; | ||
827 | req->rq_xid = xprt_alloc_xid(xprt); | 826 | req->rq_xid = xprt_alloc_xid(xprt); |
828 | req->rq_release_snd_buf = NULL; | 827 | req->rq_release_snd_buf = NULL; |
829 | xprt_reset_majortimeo(req); | 828 | xprt_reset_majortimeo(req); |