On 2021/10/25 20:55, Jason Gunthorpe wrote:
> On Fri, Oct 22, 2021 at 05:46:19PM +0800, Wenpeng Liang wrote:
>>> Just write
>>>
>>> if (entry->mmap_type != HNS_ROCE_MMAP_TYPE_TPTR)
>>> prot = pgprot_noncached(prot);
>>> ret = rdma_user_mmap_io(uctx, vma, pfn,
>>> rdma_entry->npages * PAGE_SIZE,
>>> pgprot_noncached(prot), rdma_entry);
>>>
>>> No need for the big case statement
>>>
>>
>> In the future, we will have multiple new features that will add
>> new branches to this switch. We hope to reduce the coupling
>> between subsequent patches, so we hope to keep this switch.
>
> Why? The only choice that should be made at mmap time is if this is
> noncached or not - the address and everything else should be setup
> during the creation of the entry.
>
Thanks for your comment, I will merge 'HNS_ROCE_MMAP_TYPE_TPTR' and
'HNS_ROCE_MMAP_TYPE_DB' into one statement in v3.
>>>> struct hns_roce_ib_alloc_ucontext_resp {
>>>> __u32 qp_tab_size;
>>>> __u32 cqe_size;
>>>> __u32 srq_tab_size;
>>>> - __u32 reserved;
>>>> + __u8 config;
>>>> + __u8 rsv[3];
>>>> + __aligned_u64 db_mmap_key;
>>>
>>> I'm confused, this doesn't change the uAPI, so why add this stuff?
>>> This should go in a later patch?
>>>
>>
>> The related userspace has also been modified, the link is:
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-rdma%2Fmsg106056.html&data=04%7C01%7Cjgg%40nvidia.com%7Cffac1b9f0afb458a4da308d99540ce99%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637704927843133708%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BAszHlXQWWh%2F8hrb%2F8qEStm3VBrhvbMshfPN3pc6wKI%3D&reserved=0
>>
>> I don’t know if I understood your question correctly. These fields
>> are used for compatibility, so they are necessary. The user space
>> and kernel space of different versions are handshake through 'config'.
>
> That is for later patches, this patch doesn't seem to change the uAPI
> at all. The compat mmaps remain at the same offsets they were always
> at, there is nothing to negotiate at this point.
>
> Move it to a later series?
>
> Jason
> .
>
Thanks, I will remove it in v3.
Wenpeng
