Re: [PATCH v2] IB/rxe: Fix incorrect cache cleanup in error flow

[Yanjun Zhu <yanjun.zhu@xxxxxxxxxx>]

On 2018/12/9 18:50, Yuval Shaia wrote:
> Array iterator stays at the same slot, fix it.
>
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>
> Signed-off-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
> Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> v1 -> v2:
> 	* Add Fixes tag as suggested by Parav
> 	* Add Bart's r-b
> ---
>   drivers/infiniband/sw/rxe/rxe_pool.c | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 36b53fb94a49..bf662977258e 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -112,6 +112,18 @@ static inline struct kmem_cache *pool_cache(struct rxe_pool *pool)
>   	return rxe_type_info[pool->type].cache;
>   }
>   
> +static void rxe_cache_clean(size_t to_idx)
> +{
> +	int i;
> +	struct rxe_type_info *type;
> +
> +	for (i = 0; i < to_idx; i++) {
> +		type = &rxe_type_info[i];
> +		kmem_cache_destroy(type->cache);
> +		type->cache = NULL;
> +	}
> +}
> +
>   int rxe_cache_init(void)
>   {
>   	int err;
> @@ -136,24 +148,14 @@ int rxe_cache_init(void)
>   	return 0;
>   
>   err1:
> -	while (--i >= 0) {
> -		kmem_cache_destroy(type->cache);
> -		type->cache = NULL;
> -	}
> +	rxe_cache_clean(--i);

Hi, Yuval

If i = 5,

while (--i>=0)

Here 5 times are executed.

rxe_cache_clean(--i); is sent to 4.

+    for (i = 0; i < to_idx; i++) {
+        type = &rxe_type_info[i];
+        kmem_cache_destroy(type->cache);
+        type->cache = NULL;
+    }

here 4 times are exected.

So rxe_cache_clean(i); should be correct.

After fixing that, please add:

Reviewed-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>

Zhu Yanjun

>   
>   	return err;
>   }
>   
>   void rxe_cache_exit(void)
>   {
> -	int i;
> -	struct rxe_type_info *type;
> -
> -	for (i = 0; i < RXE_NUM_TYPES; i++) {
> -		type = &rxe_type_info[i];
> -		kmem_cache_destroy(type->cache);
> -		type->cache = NULL;
> -	}
> +	rxe_cache_clean(RXE_NUM_TYPES);
>   }
>   
>   static int rxe_pool_init_index(struct rxe_pool *pool, u32 max, u32 min)