2022-05-24 1:05 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
> On 5/23/2022 11:05 AM, Namjae Jeon wrote:
>> 2022-05-23 22:45 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
>>> On 5/22/2022 7:06 PM, Namjae Jeon wrote:
>>>> 2022-05-21 20:54 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
>>>>> ...
>>>>> 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?
Yes.
> 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.
Maximum 4. (smb transform header , smb3 header+ response, data,
smb-direct header)
>
> 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.
For bulk data, ksmbd already using it.
>
> 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.
Okay.
>
> 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.
>>>
>>
>
