On Mon, Aug 24, 2020 at 01:51:11PM -0300, Jason Gunthorpe wrote:
> 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".
> >
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> > > index ccda5f5a3bc0..12c7ca0764d5 100644
> > > +++ 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.
>
> How about "Module parameters are not supported, use rdma link add"
>
I don't think so, This patch is targeted to for-rc (stable) and the
support of "rdma link add" is not part of all the stable versions.
Thanks,
Kamal
> Jason
