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).
>>> 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
