Re: [RESEND RFC PATCH for-next] Revert "RDMA/rxe: Create duplicate mapping tables for FMRs"

[Bob Pearson <rpearsonhpe@gmail.com>]

On 7/20/22 05:50, Haris Iqbal wrote:
> On Wed, Jul 20, 2022 at 12:22 PM Li Zhijian <lizhijian@fujitsu.com> wrote:
>>
>> Below 2 commits will be reverted:
>>      8ff5f5d9d8cf ("RDMA/rxe: Prevent double freeing rxe_map_set()")
>>      647bf13ce944 ("RDMA/rxe: Create duplicate mapping tables for FMRs")
>>
>> The community has a few bug reports which pointed this commit at last.
>> Some proposals are raised up in the meantime but all of them have no
>> follow-up operation.
>>
>> The previous commit led the map_set of FMR to be not avaliable any more if
>> the MR is registered again after invalidating. Although the mentioned
>> patch try to fix a potential race in building/accessing the same table
>> for fast memory regions, it broke rnbd etc ULPs. Since the latter could
>> be worse, revert this patch.
>>
>> With previous commit, it's observed that a same MR in rnbd server will
>> trigger below code path:
> 
> Looks Good. I tested the patch against rdma for-next and it solves the
> problem mentioned in the commit.
> One small nitpick. It should be rtrs, and not rnbd in the commit message.
> 
> Feel free to add my,
> 
> Tested-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> 
>>  -> rxe_mr_init_fast()
>>  |-> alloc map_set() # map_set is uninitialized
>>  |...-> rxe_map_mr_sg() # build the map_set
>>      |-> rxe_mr_set_page()
>>  |...-> rxe_reg_fast_mr() # mr->state change to VALID from FREE that means
>>                           # we can access host memory(such rxe_mr_copy)
>>  |...-> rxe_invalidate_mr() # mr->state change to FREE from VALID
>>  |...-> rxe_reg_fast_mr() # mr->state change to VALID from FREE,
>>                           # but map_set was not built again
>>  |...-> rxe_mr_copy() # kernel crash due to access wild addresses
>>                       # that lookup from the map_set
>>

Where is the use case for this? All the FMR examples I am aware of call rxe_map_mr_sg()
between each reg_fast_mr/invalidate_mr() sequence. I am not familiar with rtrs.
What is it?

Bob