Hi Bob
I just post a V2 by following your suggestions. PTAL.
Thanks
Zhijian
On 27/12/2021 10:53, Li Zhijian wrote:
>
>
> On 25/12/2021 03:29, Bob Pearson wrote:
>> Somehow I lost the email so you could resend it that would help.
>>
>> Thanks. Good catch. I think there is a cleaner and simpler way to fix this. All the errors occur when there is a failure in setting up new MRs which first try to delete the already allocated memory and then drop the object which cleans it up again.
>
>> It would be simpler to just return an error and not call rxe_mr_free_map_set(). Then the error will lead to rxe_mr_cleanup() which will delete the memory just once.
> Not sure i have got you.
> i think it's not a convention style that we don't release/cleanup allocated resources in error path in a same function.
> rxe_mr_alloc() will release the map set itself in error path, where it also doesn't set map_set to NULL. that may also
> cause a double free.
>
> I wonder if you suggested that:
> :~/lkp/linux$ git diff
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 53271df10e47..a08b4718feec 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -140,7 +140,6 @@ static int rxe_mr_alloc(struct rxe_mr *mr, int num_buf, int both)
> if (both) {
> ret = rxe_mr_alloc_map_set(num_map, &mr->next_map_set);
> if (ret) {
> - rxe_mr_free_map_set(mr->num_map, mr->cur_map_set);
> goto err_out;
> }
> }
>
>
> Thanks
> Zhijian
>
>>
>> Bob
>
