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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 25, 2020 at 10:25:57AM -0300, Jason Gunthorpe wrote:
> On Tue, Aug 25, 2020 at 12:36:24PM +0300, Kamal Heib wrote:
> > 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.
> 
> then add "or rxe_cfg"
>
Done.

Thanks,
Kamal

> Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux