Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-13 Thread Chuck Lever 

On Jul 10, 2015, at 12:05 PM, J. Bruce Fields  wrote:

> On Fri, Jul 10, 2015 at 11:59:20AM -0400, Chuck Lever wrote:
>> 
>> On Jul 10, 2015, at 11:54 AM, J. Bruce Fields  wrote:
>> 
>>> On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
 
 On Jul 10, 2015, at 10:18 AM, bfie...@fieldses.org wrote:
 
> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>> Increased to 1 megabyte.
> 
> Why not more or less?
> 
> Why do we even have this constant, why shouldn't we just use
> RPCSVC_MAXPAYLOAD?
 
 The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
 We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
 based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
>>> 
>>> But there'd be no reason to do that, because we're not using
>>> RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?
>> 
>> Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
>> which is actually used in svc_rdma_*.c
> 
> OK, thanks, that sounds like that would make more sense.

The change to use RPCSVC_MAXPAYLOAD is clean enough, but it may
re-expose a PPC64-x86_64 interop bug. We need to dig up some
testing resources to see if that bug is still a problem. Stay
tuned.

Since you’ve taken the other patches in this series, I’ll hold
off on reposting a v2 until I have this straightened out.


> --b.
> 
>> 
>> 
>>> --b.
>>> 
 
 
> --b.
> 
>> 
>> Signed-off-by: Chuck Lever 
>> ---
>> 
>> include/linux/sunrpc/svc_rdma.h |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/include/linux/sunrpc/svc_rdma.h 
>> b/include/linux/sunrpc/svc_rdma.h
>> index 13af61b..1bca6dd 100644
>> --- a/include/linux/sunrpc/svc_rdma.h
>> +++ b/include/linux/sunrpc/svc_rdma.h
>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>> #define RDMAXPRT_SQ_PENDING  2
>> #define RDMAXPRT_CONN_PENDING3
>> 
>> -#define RPCRDMA_MAX_SVC_SEGS(64)/* server max scatter/gather */
>> +#define RPCRDMA_MAX_SVC_SEGS(256)   /* server max scatter/gather */
>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>> #define RPCRDMA_MAXPAYLOAD   RPCSVC_MAXPAYLOAD
>> #else
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 --
 Chuck Lever
 
 
>> 
>> --
>> Chuck Lever
>> 
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread J. Bruce Fields 
On Fri, Jul 10, 2015 at 11:59:20AM -0400, Chuck Lever wrote:
> 
> On Jul 10, 2015, at 11:54 AM, J. Bruce Fields  wrote:
> 
> > On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
> >> 
> >> On Jul 10, 2015, at 10:18 AM, bfie...@fieldses.org wrote:
> >> 
> >>> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>  Increased to 1 megabyte.
> >>> 
> >>> Why not more or less?
> >>> 
> >>> Why do we even have this constant, why shouldn't we just use
> >>> RPCSVC_MAXPAYLOAD?
> >> 
> >> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
> >> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
> >> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
> > 
> > But there'd be no reason to do that, because we're not using
> > RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?
> 
> Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
> which is actually used in svc_rdma_*.c

OK, thanks, that sounds like that would make more sense.

--b.

> 
> 
> > --b.
> > 
> >> 
> >> 
> >>> --b.
> >>> 
>  
>  Signed-off-by: Chuck Lever 
>  ---
>  
>  include/linux/sunrpc/svc_rdma.h |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>  
>  diff --git a/include/linux/sunrpc/svc_rdma.h 
>  b/include/linux/sunrpc/svc_rdma.h
>  index 13af61b..1bca6dd 100644
>  --- a/include/linux/sunrpc/svc_rdma.h
>  +++ b/include/linux/sunrpc/svc_rdma.h
>  @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>  #define RDMAXPRT_SQ_PENDING  2
>  #define RDMAXPRT_CONN_PENDING3
>  
>  -#define RPCRDMA_MAX_SVC_SEGS(64)/* server max scatter/gather */
>  +#define RPCRDMA_MAX_SVC_SEGS(256)   /* server max scatter/gather */
>  #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>  #define RPCRDMA_MAXPAYLOAD   RPCSVC_MAXPAYLOAD
>  #else
>  
>  --
>  To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>  the body of a message to majord...@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> 
> >> --
> >> Chuck Lever
> >> 
> >> 
> 
> --
> Chuck Lever
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread Chuck Lever 

On Jul 10, 2015, at 11:54 AM, J. Bruce Fields  wrote:

> On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
>> 
>> On Jul 10, 2015, at 10:18 AM, bfie...@fieldses.org wrote:
>> 
>>> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
 Increased to 1 megabyte.
>>> 
>>> Why not more or less?
>>> 
>>> Why do we even have this constant, why shouldn't we just use
>>> RPCSVC_MAXPAYLOAD?
>> 
>> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
>> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
>> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
> 
> But there'd be no reason to do that, because we're not using
> RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?

Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
which is actually used in svc_rdma_*.c


> --b.
> 
>> 
>> 
>>> --b.
>>> 
 
 Signed-off-by: Chuck Lever 
 ---
 
 include/linux/sunrpc/svc_rdma.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/include/linux/sunrpc/svc_rdma.h 
 b/include/linux/sunrpc/svc_rdma.h
 index 13af61b..1bca6dd 100644
 --- a/include/linux/sunrpc/svc_rdma.h
 +++ b/include/linux/sunrpc/svc_rdma.h
 @@ -172,7 +172,7 @@ struct svcxprt_rdma {
 #define RDMAXPRT_SQ_PENDING2
 #define RDMAXPRT_CONN_PENDING  3
 
 -#define RPCRDMA_MAX_SVC_SEGS  (64)/* server max scatter/gather */
 +#define RPCRDMA_MAX_SVC_SEGS  (256)   /* server max scatter/gather */
 #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
 #define RPCRDMA_MAXPAYLOAD RPCSVC_MAXPAYLOAD
 #else
 
 --
 To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> Chuck Lever
>> 
>> 

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread J. Bruce Fields 
On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
> 
> On Jul 10, 2015, at 10:18 AM, bfie...@fieldses.org wrote:
> 
> > On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> >> Increased to 1 megabyte.
> > 
> > Why not more or less?
> > 
> > Why do we even have this constant, why shouldn't we just use
> > RPCSVC_MAXPAYLOAD?
> 
> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.

But there'd be no reason to do that, because we're not using
RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?

--b.

> 
> 
> > --b.
> > 
> >> 
> >> Signed-off-by: Chuck Lever 
> >> ---
> >> 
> >> include/linux/sunrpc/svc_rdma.h |2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/include/linux/sunrpc/svc_rdma.h 
> >> b/include/linux/sunrpc/svc_rdma.h
> >> index 13af61b..1bca6dd 100644
> >> --- a/include/linux/sunrpc/svc_rdma.h
> >> +++ b/include/linux/sunrpc/svc_rdma.h
> >> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
> >> #define RDMAXPRT_SQ_PENDING2
> >> #define RDMAXPRT_CONN_PENDING  3
> >> 
> >> -#define RPCRDMA_MAX_SVC_SEGS  (64)/* server max scatter/gather */
> >> +#define RPCRDMA_MAX_SVC_SEGS  (256)   /* server max scatter/gather */
> >> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
> >> #define RPCRDMA_MAXPAYLOAD RPCSVC_MAXPAYLOAD
> >> #else
> >> 
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majord...@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread Chuck Lever 

On Jul 10, 2015, at 10:18 AM, bfie...@fieldses.org wrote:

> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>> Increased to 1 megabyte.
> 
> Why not more or less?
> 
> Why do we even have this constant, why shouldn't we just use
> RPCSVC_MAXPAYLOAD?

The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.


> --b.
> 
>> 
>> Signed-off-by: Chuck Lever 
>> ---
>> 
>> include/linux/sunrpc/svc_rdma.h |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/include/linux/sunrpc/svc_rdma.h 
>> b/include/linux/sunrpc/svc_rdma.h
>> index 13af61b..1bca6dd 100644
>> --- a/include/linux/sunrpc/svc_rdma.h
>> +++ b/include/linux/sunrpc/svc_rdma.h
>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>> #define RDMAXPRT_SQ_PENDING  2
>> #define RDMAXPRT_CONN_PENDING3
>> 
>> -#define RPCRDMA_MAX_SVC_SEGS(64)/* server max scatter/gather */
>> +#define RPCRDMA_MAX_SVC_SEGS(256)   /* server max scatter/gather */
>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>> #define RPCRDMA_MAXPAYLOAD   RPCSVC_MAXPAYLOAD
>> #else
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread J. Bruce Fields 
On Fri, Jul 10, 2015 at 10:18:14AM -0400, J. Bruce Fields wrote:
> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> > Increased to 1 megabyte.
> 
> Why not more or less?
> 
> Why do we even have this constant, why shouldn't we just use
> RPCSVC_MAXPAYLOAD?

(That one question aside these look fine, I'll apply unless I hear
otherwise.)

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-10 Thread J. Bruce Fields 
On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> Increased to 1 megabyte.

Why not more or less?

Why do we even have this constant, why shouldn't we just use
RPCSVC_MAXPAYLOAD?

--b.

> 
> Signed-off-by: Chuck Lever 
> ---
> 
>  include/linux/sunrpc/svc_rdma.h |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> index 13af61b..1bca6dd 100644
> --- a/include/linux/sunrpc/svc_rdma.h
> +++ b/include/linux/sunrpc/svc_rdma.h
> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>  #define RDMAXPRT_SQ_PENDING  2
>  #define RDMAXPRT_CONN_PENDING3
>  
> -#define RPCRDMA_MAX_SVC_SEGS (64)/* server max scatter/gather */
> +#define RPCRDMA_MAX_SVC_SEGS (256)   /* server max scatter/gather */
>  #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>  #define RPCRDMA_MAXPAYLOAD   RPCSVC_MAXPAYLOAD
>  #else
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

2015-07-09 Thread Chuck Lever 
Increased to 1 megabyte.

Signed-off-by: Chuck Lever 
---

 include/linux/sunrpc/svc_rdma.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 13af61b..1bca6dd 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -172,7 +172,7 @@ struct svcxprt_rdma {
 #define RDMAXPRT_SQ_PENDING2
 #define RDMAXPRT_CONN_PENDING  3
 
-#define RPCRDMA_MAX_SVC_SEGS   (64)/* server max scatter/gather */
+#define RPCRDMA_MAX_SVC_SEGS   (256)   /* server max scatter/gather */
 #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
 #define RPCRDMA_MAXPAYLOAD RPCSVC_MAXPAYLOAD
 #else

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html