Re: RDMA (smbdirect) testing

[Tom Talpey <tom@talpey.com>]

On 5/23/2022 11:05 AM, Namjae Jeon wrote:
> 2022-05-23 22:45 GMT+09:00, Tom Talpey <tom@talpey.com>:
>> On 5/22/2022 7:06 PM, Namjae Jeon wrote:
>>> 2022-05-21 20:54 GMT+09:00, Tom Talpey <tom@talpey.com>:
>>>> ...
>>>> Why does the code require
>>>> 16 sge's, regardless of other size limits? Normally, if the lower layer
>>>> supports fewer, the upper layer will simply reduce its operation sizes.
>>> This should be answered by Long Li. It seems that he set the optimized
>>> value for the NICs he used to implement RDMA in cifs.
>>
>> "Optimized" is a funny choice of words. If the provider doesn't support
>> the value, it's not much of an optimization to insist on 16. :)
> Ah, It's obvious that cifs haven't been tested with soft-iWARP. And
> the same with ksmbd...
>>
>> Personally, I'd try building a kernel with smbdirect.h changed to have
>> SMBDIRECT_MAX_SGE set to 6, and see what happens. You might have to
>> reduce the r/w sizes in mount, depending on any other issues this may
>> reveal.
> Agreed, and ksmbd should also be changed as well as cifs for test. We
> are preparing the patches to improve this in ksmbd, rather than
> changing/building this hardcoding every time.

So, the patch is just for this test, right? Because I don't think any
kernel-based storage upper layer should ever need more than 2 or 3.
How many memory regions are you doing per operation? I would
expect one for the SMB3 headers, and another, if needed, for data.
These would all be lmr-type and would not require actual new memreg's.

And for bulk data, I would hope you're using fast-register, which
takes a different path and doesn't use the same sge's.

Getting this right, and keeping things efficient both in SGE bookkeeping
as well as memory registration efficiency, is the rocket science behind
RDMA performance and correctness. Slapping "16" or "6" or whatever isn't
the long-term fix.

Tom.

> diff --git a/fs/cifs/smbdirect.h b/fs/cifs/smbdirect.h
> index a87fca82a796..7003722ab004 100644
> --- a/fs/cifs/smbdirect.h
> +++ b/fs/cifs/smbdirect.h
> @@ -226,7 +226,7 @@ struct smbd_buffer_descriptor_v1 {
>   } __packed;
> 
>   /* Default maximum number of SGEs in a RDMA send/recv */
> -#define SMBDIRECT_MAX_SGE      16
> +#define SMBDIRECT_MAX_SGE      6
>   /* The context for a SMBD request */
>   struct smbd_request {
>          struct smbd_connection *info;
> diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c
> index e646d79554b8..70662b3bd590 100644
> --- a/fs/ksmbd/transport_rdma.c
> +++ b/fs/ksmbd/transport_rdma.c
> @@ -42,7 +42,7 @@
>   /* SMB_DIRECT negotiation timeout in seconds */
>   #define SMB_DIRECT_NEGOTIATE_TIMEOUT           120
> 
> -#define SMB_DIRECT_MAX_SEND_SGES               8
> +#define SMB_DIRECT_MAX_SEND_SGES               6
>   #define SMB_DIRECT_MAX_RECV_SGES               1
> 
>   /*
> 
> Thanks!
>>
>> Tom.
>>
>