Re: [RFC PATCH rdma-next 08/10] RDMA/rxe: Implement flush execution in responder side

From: lizhijian@xxxxxxxxxxx
Date: Tue Jan 11 2022 - 00:34:51 EST




On 10/01/2022 22:34, Jason Gunthorpe wrote:
> On Mon, Jan 10, 2022 at 05:45:47AM +0000, lizhijian@xxxxxxxxxxx wrote:
>> Hi Jason
>>
>>
>> On 07/01/2022 01:33, Jason Gunthorpe wrote:
>>> On Thu, Jan 06, 2022 at 06:42:57AM +0000, lizhijian@xxxxxxxxxxx wrote:
>>>> On 06/01/2022 08:28, Jason Gunthorpe wrote:
>>>>> On Tue, Dec 28, 2021 at 04:07:15PM +0800, Li Zhijian wrote:
>>>>>> + while (length > 0) {
>>>>>> + va = (u8 *)(uintptr_t)buf->addr + offset;
>>>>>> + bytes = buf->size - offset;
>>>>>> +
>>>>>> + if (bytes > length)
>>>>>> + bytes = length;
>>>>>> +
>>>>>> + arch_wb_cache_pmem(va, bytes);
>>>>> So why did we need to check that the va was pmem to call this?
>>>> Sorry, i didn't get you.
>>>>
>>>> I didn't check whether va is pmem, since only MR registered with PERSISTENCE(only pmem can
>>>> register this access flag) can reach here.
>>> Yes, that is what I mean,
>> I'm not sure I understand the *check* you mentioned above.
>>
>> Current code just dose something like:
>>
>> if (!sanity_check())
>>     return;
>> if (requested_plt == PERSISTENCE)
>>     va = iova_to_va(iova);
>>     arch_wb_cache_pmem(va, bytes);
>>     wmb;
>> else if (requested_plt == GLOBAL_VISIBILITY)
>>     wmb();
>>
>>
>>> why did we need to check anything to call
>>> this API
>> As above pseudo code,  it didn't *check* anything as what you said i think.
> I mean when you created the MR in the first place you checked for pmem
> before even allowing the persitent access flag.

Yes, that's true. that's because only pmem has ability to persist data.
So do you mean we don't need to prevent user to create/register a persistent
access flag to a non-pmem MR? it would be a bit confusing if so.


>
> Jason