Re: [PATCH 1/1] RDMA/rxe: Fix blktests srp lead kernel panic with 64k page size

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 2023/10/23 11:52, Zhijian Li (Fujitsu) 写道:
> 
> 
> 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@linux.dev>
>>>>>
>>>>> 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@OS3PR01MB9865.jpnprd01.prod.outlook.com/T/#md133060414f0ba6a3dbaf7b4ad2374c8a347cfd1

About this patch, please make full tests. Please ensure that this will 
not introduce some potential problems. In the discussion, this patch 
possibly has side effect.

Zhu Yanjun

> 
> 
>> Jason