Re: [PATCH 1/2] RDMA: RPCRDMA_MAX_DATA_SEGS cause i686 stack to blow.

[Boaz Harrosh <bharrosh@panasas.com>]

On 11/03/2011 11:52 AM, Steve Dickson wrote:
> Commit 2773395b3 increased RPCRDMA_MAX_DATA_SEGS from 8
> to 64 which significantly increases throughput with the
> RDMA transport.
> 
> Unfortunately making RPCRDMA_MAX_DATA_SEGS that large blows
> the stack on i686 builds during compilation. So the
> RPCRDMA_MAX_PHYSREG_DATA_SEGS is used to allocated data on
> the stack.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  net/sunrpc/xprtrdma/verbs.c     |    6 +++---
>  net/sunrpc/xprtrdma/xprt_rdma.h |    1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 28236ba..68ba0fb 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -1729,11 +1729,11 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
>  	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
>  				  IB_ACCESS_REMOTE_READ);
>  	struct rpcrdma_mr_seg *seg1 = seg;
> -	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
> +	struct ib_phys_buf ipb[RPCRDMA_MAX_PHYSREG_DATA_SEGS];

Can't you just allocate this buffer on heap? Any size of a big array
on the stack is a bad idea. And it does help performance as reported
by original commit

Boaz
>  	int len, i, rc = 0;
>  
> -	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
> -		*nsegs = RPCRDMA_MAX_DATA_SEGS;
> +	if (*nsegs > RPCRDMA_MAX_PHYSREG_DATA_SEGS)
> +		*nsegs = RPCRDMA_MAX_PHYSREG_DATA_SEGS;
>  	for (len = 0, i = 0; i < *nsegs;) {
>  		rpcrdma_map_one(ia, seg, writing);
>  		ipb[i].addr = seg->mr_dma;
> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
> index 08c5d5a..8d588f2 100644
> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
> @@ -110,6 +110,7 @@ struct rpcrdma_ep {
>  
>  /* temporary static scatter/gather max */
>  #define RPCRDMA_MAX_DATA_SEGS	(64)	/* max scatter/gather */
> +#define RPCRDMA_MAX_PHYSREG_DATA_SEGS	(16)
>  #define RPCRDMA_MAX_SEGS 	(RPCRDMA_MAX_DATA_SEGS + 2) /* head+tail = 2 */
>  #define MAX_RPCRDMAHDR	(\
>  	/* max supported RPC/RDMA header */ \