Re: [PATCH v3 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()

[Kamal Heib <kamalheib1@gmail.com>]

On Mon, Aug 24, 2020 at 09:41:14AM -0700, Bart Van Assche wrote:
> On 8/24/20 8:52 AM, Kamal Heib wrote:
> > +bool rxe_is_loaded;
> 
> The name of this variable seems wrong to me. My understanding is that rxe_module_init() is
> called whether or not rxe has been built as a module. Consider renaming this variable into
> e.g. "rxe_initialized".
>
OK, I'll change it.

> > diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> > index ccda5f5a3bc0..12c7ca0764d5 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> > @@ -61,6 +61,11 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
> >   	struct net_device *ndev;
> >   	struct rxe_dev *exists;
> > +	if (!rxe_is_loaded) {
> > +		pr_err("Please make sure to load the rdma_rxe module first\n");
> > +		return -EINVAL;
> > +	}
> > +
> >   	len = sanitize_arg(val, intf, sizeof(intf));
> >   	if (!len) {
> >   		pr_err("add: invalid interface name\n");
> 
> The above message is misleading. Consider changing it into e.g. the following:
> 
>     Please wait until initialization of the rdma_rxe module has finished.
> 
> Additionally, how about returning -EAGAIN instead of -EINVAL?
>
Yes, this makes more sense, I'll change it.

Thanks,
Kamal

> Thanks,
> 
> Bart.
> 
>