On 23/10/2023 18:45, Yi Zhang wrote:
> On Mon, Oct 23, 2023 at 11:52 AM Zhijian Li (Fujitsu)
> <lizhijian@xxxxxxxxxxx> wrote:
>>
>>
>>
>> On 20/10/2023 22:01, Jason Gunthorpe wrote:
>>> On Fri, Oct 20, 2023 at 03:47:05AM +0000, Zhijian Li (Fujitsu) wrote:
>>>> CC Bart
>>>>
>>>> On 13/10/2023 20:01, Daisuke Matsuda (Fujitsu) wrote:
>>>>> On Fri, Oct 13, 2023 10:18 AM Zhu Yanjun wrote:
>>>>>> From: Zhu Yanjun<yanjun.zhu@xxxxxxxxx>
>>>>>>
>>>>>> The page_size of mr is set in infiniband core originally. In the commit
>>>>>> 325a7eb85199 ("RDMA/rxe: Cleanup page variables in rxe_mr.c"), the
>>>>>> page_size is also set. Sometime this will cause conflict.
>>>>> I appreciate your prompt action, but I do not think this commit deals with
>>>>> the root cause. I agree that the problem lies in rxe driver, but what is wrong
>>>>> with assigning actual page size to ibmr.page_size?
>>>>>
>>>>> IMO, the problem comes from the device attribute of rxe driver, which is used
>>>>> in ulp/srp layer to calculate the page_size.
>>>>> =====
>>>>> static int srp_add_one(struct ib_device *device)
>>>>> {
>>>>> struct srp_device *srp_dev;
>>>>> struct ib_device_attr *attr = &device->attrs;
>>>>> <...>
>>>>> /*
>>>>> * Use the smallest page size supported by the HCA, down to a
>>>>> * minimum of 4096 bytes. We're unlikely to build large sglists
>>>>> * out of smaller entries.
>>>>> */
>>>>> mr_page_shift = max(12, ffs(attr->page_size_cap) - 1);
>>>>
>>>>
>>>> You light me up.
>>>> RXE provides attr.page_size_cap(RXE_PAGE_SIZE_CAP) which means it can support 4K-2G page size
>>>
>>> That doesn't seem right even in concept.>
>>> I think the multi-size support in the new xarray code does not work
>>> right, just looking at it makes me think it does not work right. It
>>> looks like it can do less than PAGE_SIZE but more than PAGE_SIZE will
>>> explode because kmap_local_page() does only 4K.
>>>
>>> If RXE_PAGE_SIZE_CAP == PAGE_SIZE will everything work?
>>>
>>
>> Yeah, this should work(even though i only verified hardcoding mr_page_shift to PAGE_SHIFT).
>
> Hi Zhijian
>
> Did you try blktests nvme/rdma use_rxe on your environment, it still
> failed on my side.
>
Thanks for your testing.
I just did that, it also failed in my environment. After a glance debug, I found there are
other places still registered 4K MR. I will dig into it later.
Thanks
Zhijian
> # use_rxe=1 nvme_trtype=rdma ./check nvme/003
> nvme/003 (test if we're sending keep-alives to a discovery controller) [failed]
> runtime 12.179s ... 11.941s
> --- tests/nvme/003.out 2023-10-22 10:54:43.041749537 -0400
> +++ /root/blktests/results/nodev/nvme/003.out.bad 2023-10-23
> 05:52:27.882759168 -0400
> @@ -1,3 +1,3 @@
> Running nvme/003
> -NQN:nqn.2014-08.org.nvmexpress.discovery disconnected 1 controller(s)
> +NQN:nqn.2014-08.org.nvmexpress.discovery disconnected 0 controller(s)
> Test complete
>
> [ 7033.431910] rdma_rxe: loaded
> [ 7033.456341] run blktests nvme/003 at 2023-10-23 05:52:15
> [ 7033.502306] (null): rxe_set_mtu: Set mtu to 1024
> [ 7033.510969] infiniband enP2p1s0v0_rxe: set active
> [ 7033.510980] infiniband enP2p1s0v0_rxe: added enP2p1s0v0
> [ 7033.549301] loop0: detected capacity change from 0 to 2097152
> [ 7033.556966] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
> [ 7033.566711] nvmet_rdma: enabling port 0 (10.19.240.81:4420)
> [ 7033.588605] nvmet: connect attempt for invalid controller ID 0x808
> [ 7033.594909] nvme nvme0: Connect Invalid Data Parameter, cntlid: 65535
> [ 7033.601504] nvme nvme0: failed to connect queue: 0 ret=16770
> [ 7046.317861] rdma_rxe: unloaded
>
>
>>
>>>>> import ctypes
>>>>> libc = ctypes.cdll.LoadLibrary('libc.so.6')
>>>>> hex(65536)
>> '0x10000'
>>>>> libc.ffs(0x10000) - 1
>> 16
>>>>> 1 << 16
>> 65536
>>
>> so
>> mr_page_shift = max(12, ffs(attr->page_size_cap) - 1) = max(12, 16) = 16;
>>
>>
>> So I think Daisuke's patch should work as well.
>>
>> https://lore.kernel.org/linux-rdma/OS3PR01MB98652B2EC2E85DAEC6DDE484E5D2A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#md133060414f0ba6a3dbaf7b4ad2374c8a347cfd1
>>
>>
>>> Jason
>
>
>
> --
> Best Regards,
> Yi Zhang
>
