Re: [PATCH rdma-next 2/5] RDMA: Clean MW allocation and free flows

[Leon Romanovsky <leon@kernel.org>]

On Sun, Jun 28, 2020 at 12:18:38PM +0300, Yishai Hadas wrote:
> On 6/24/2020 1:54 PM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Move allocation and destruction of memory windows under ib_core
> > responsibility and clean drivers to ensure that no updates to MW
> > ib_core structures are done in driver layer.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---

<...>

> > +void mlx5_ib_dealloc_mw(struct ib_mw *mw)
> >   {
> >   	struct mlx5_ib_dev *dev = to_mdev(mw->device);
> >   	struct mlx5_ib_mw *mmw = to_mmw(mw);
> > -	int err;
> >   	if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING)) {
> >   		xa_erase(&dev->odp_mkeys, mlx5_base_mkey(mmw->mmkey.key));
> > @@ -2073,11 +2067,7 @@ int mlx5_ib_dealloc_mw(struct ib_mw *mw)
> >   		synchronize_srcu(&dev->odp_srcu);
> >   	}
> > -	err = mlx5_core_destroy_mkey(dev->mdev, &mmw->mmkey);
> > -	if (err)
> > -		return err;
> > -	kfree(mmw);
> > -	return 0;
> > +	mlx5_core_destroy_mkey(dev->mdev, &mmw->mmkey);
>
> Are we fully sure that this can never be triggered by user space to fail as
> of the property of the MW that can be binded with bypassing kernel ? the new
> code just ignored the err.

Why is it different from any other HW object?
The failure to destroy will leave leaked kernel resources.

We already removed this mmkey from xarray above and if we don't finish,
the MW will be leaked.

Thanks