| Message ID | 20180612073420.45736-2-hare@suse.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
On Tue, Jun 12, 2018 at 10:35 AM Hannes Reinecke <hare@suse.de> wrote: > > When using the module option 'add' during modprobe the module will crash > as the option is evaluated before the module itself is initialized. > So return an error when rxe_param_set_add() is called before the module > is initialized. > > Signed-off-by: Hannes Reinecke <hare@suse.com> > --- > drivers/infiniband/sw/rxe/rxe.c | 6 ++++++ > drivers/infiniband/sw/rxe/rxe.h | 2 ++ > drivers/infiniband/sw/rxe/rxe_sysfs.c | 5 +++++ > 3 files changed, 13 insertions(+) > > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > index 7121e1b1eb89..53279d82e5cc 100644 > --- a/drivers/infiniband/sw/rxe/rxe.c > +++ b/drivers/infiniband/sw/rxe/rxe.c > @@ -39,6 +39,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib"); > MODULE_DESCRIPTION("Soft RDMA transport"); > MODULE_LICENSE("Dual BSD/GPL"); > > +bool rxe_init_done = false; > + > /* free resources for all ports on a device */ > static void rxe_cleanup_ports(struct rxe_dev *rxe) > { > @@ -354,6 +356,8 @@ static int __init rxe_module_init(void) > if (err) > return err; > > + rxe_init_done = true; > + > pr_info("loaded\n"); > return 0; > } > @@ -364,6 +368,8 @@ static void __exit rxe_module_exit(void) > rxe_net_exit(); > rxe_cache_exit(); > > + rxe_init_done = false; why not earlier? > + > pr_info("unloaded\n"); > } > > diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h > index d9ec2de68738..0cf68bc7aa01 100644 > --- a/drivers/infiniband/sw/rxe/rxe.h > +++ b/drivers/infiniband/sw/rxe/rxe.h > @@ -92,6 +92,8 @@ static inline u32 rxe_crc32(struct rxe_dev *rxe, > return retval; > } > > +extern bool rxe_init_done; > + > void rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu); > > int rxe_add(struct rxe_dev *rxe, unsigned int mtu); > diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c > index d5ed7571128f..bb54992e3ff2 100644 > --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c > +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c > @@ -77,6 +77,11 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp) > struct net_device *ndev = NULL; > struct rxe_dev *rxe; > > + if (!rxe_init_done) { > + pr_err("add: module not initializes\n"); > + err = -EINVAL; I think that EAGAIN is better > + goto err; > + } > len = sanitize_arg(val, intf, sizeof(intf)); > if (!len) { > pr_err("add: invalid interface name\n"); > -- > 2.12.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 12 Jun 2018 11:06:15 +0300 Moni Shoua <monis@mellanox.com> wrote: > On Tue, Jun 12, 2018 at 10:35 AM Hannes Reinecke <hare@suse.de> wrote: > > > > When using the module option 'add' during modprobe the module will > > crash as the option is evaluated before the module itself is > > initialized. So return an error when rxe_param_set_add() is called > > before the module is initialized. > > > > Signed-off-by: Hannes Reinecke <hare@suse.com> > > --- > > drivers/infiniband/sw/rxe/rxe.c | 6 ++++++ > > drivers/infiniband/sw/rxe/rxe.h | 2 ++ > > drivers/infiniband/sw/rxe/rxe_sysfs.c | 5 +++++ > > 3 files changed, 13 insertions(+) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe.c > > b/drivers/infiniband/sw/rxe/rxe.c index 7121e1b1eb89..53279d82e5cc > > 100644 --- a/drivers/infiniband/sw/rxe/rxe.c > > +++ b/drivers/infiniband/sw/rxe/rxe.c > > @@ -39,6 +39,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John > > Groves, Kamal Heib"); MODULE_DESCRIPTION("Soft RDMA transport"); > > MODULE_LICENSE("Dual BSD/GPL"); > > > > +bool rxe_init_done = false; > > + > > /* free resources for all ports on a device */ > > static void rxe_cleanup_ports(struct rxe_dev *rxe) > > { > > @@ -354,6 +356,8 @@ static int __init rxe_module_init(void) > > if (err) > > return err; > > > > + rxe_init_done = true; > > + > > pr_info("loaded\n"); > > return 0; > > } > > @@ -364,6 +368,8 @@ static void __exit rxe_module_exit(void) > > rxe_net_exit(); > > rxe_cache_exit(); > > > > + rxe_init_done = false; > why not earlier? Because it's impossible to call 'modprobe' while an 'rmmod' is running; they are serialized against each other so we can be sure the module is fully unloaded before modprobe is run again. > > + > > pr_info("unloaded\n"); > > } > > > > diff --git a/drivers/infiniband/sw/rxe/rxe.h > > b/drivers/infiniband/sw/rxe/rxe.h index d9ec2de68738..0cf68bc7aa01 > > 100644 --- a/drivers/infiniband/sw/rxe/rxe.h > > +++ b/drivers/infiniband/sw/rxe/rxe.h > > @@ -92,6 +92,8 @@ static inline u32 rxe_crc32(struct rxe_dev *rxe, > > return retval; > > } > > > > +extern bool rxe_init_done; > > + > > void rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu); > > > > int rxe_add(struct rxe_dev *rxe, unsigned int mtu); > > diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c > > b/drivers/infiniband/sw/rxe/rxe_sysfs.c index > > d5ed7571128f..bb54992e3ff2 100644 --- > > a/drivers/infiniband/sw/rxe/rxe_sysfs.c +++ > > b/drivers/infiniband/sw/rxe/rxe_sysfs.c @@ -77,6 +77,11 @@ static > > int rxe_param_set_add(const char *val, const struct kernel_param > > *kp) struct net_device *ndev = NULL; struct rxe_dev *rxe; > > > > + if (!rxe_init_done) { > > + pr_err("add: module not initializes\n"); > > + err = -EINVAL; > I think that EAGAIN is better I don't mind. Will be resending the patch. Cheers, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 12, 2018 at 11:40 AM Hannes Reinecke <hare@suse.de> wrote: > > On Tue, 12 Jun 2018 11:06:15 +0300 > Moni Shoua <monis@mellanox.com> wrote: > > > On Tue, Jun 12, 2018 at 10:35 AM Hannes Reinecke <hare@suse.de> wrote: > > > > > > When using the module option 'add' during modprobe the module will > > > crash as the option is evaluated before the module itself is > > > initialized. So return an error when rxe_param_set_add() is called > > > before the module is initialized. > > > > > > Signed-off-by: Hannes Reinecke <hare@suse.com> > > > --- > > > drivers/infiniband/sw/rxe/rxe.c | 6 ++++++ > > > drivers/infiniband/sw/rxe/rxe.h | 2 ++ > > > drivers/infiniband/sw/rxe/rxe_sysfs.c | 5 +++++ > > > 3 files changed, 13 insertions(+) > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe.c > > > b/drivers/infiniband/sw/rxe/rxe.c index 7121e1b1eb89..53279d82e5cc > > > 100644 --- a/drivers/infiniband/sw/rxe/rxe.c > > > +++ b/drivers/infiniband/sw/rxe/rxe.c > > > @@ -39,6 +39,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John > > > Groves, Kamal Heib"); MODULE_DESCRIPTION("Soft RDMA transport"); > > > MODULE_LICENSE("Dual BSD/GPL"); > > > > > > +bool rxe_init_done = false; > > > + > > > /* free resources for all ports on a device */ > > > static void rxe_cleanup_ports(struct rxe_dev *rxe) > > > { > > > @@ -354,6 +356,8 @@ static int __init rxe_module_init(void) > > > if (err) > > > return err; > > > > > > + rxe_init_done = true; > > > + > > > pr_info("loaded\n"); > > > return 0; > > > } > > > @@ -364,6 +368,8 @@ static void __exit rxe_module_exit(void) > > > rxe_net_exit(); > > > rxe_cache_exit(); > > > > > > + rxe_init_done = false; > > why not earlier? > > Because it's impossible to call 'modprobe' while an 'rmmod' is running; > they are serialized against each other so we can be sure the module is > fully unloaded before modprobe is run again. > but still, you don't want any add operations to start when rmmod started. In fact, I think that the rmmod flow might be racy and you'd like to handle it with more care. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index 7121e1b1eb89..53279d82e5cc 100644 --- a/drivers/infiniband/sw/rxe/rxe.c +++ b/drivers/infiniband/sw/rxe/rxe.c @@ -39,6 +39,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib"); MODULE_DESCRIPTION("Soft RDMA transport"); MODULE_LICENSE("Dual BSD/GPL"); +bool rxe_init_done = false; + /* free resources for all ports on a device */ static void rxe_cleanup_ports(struct rxe_dev *rxe) { @@ -354,6 +356,8 @@ static int __init rxe_module_init(void) if (err) return err; + rxe_init_done = true; + pr_info("loaded\n"); return 0; } @@ -364,6 +368,8 @@ static void __exit rxe_module_exit(void) rxe_net_exit(); rxe_cache_exit(); + rxe_init_done = false; + pr_info("unloaded\n"); } diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h index d9ec2de68738..0cf68bc7aa01 100644 --- a/drivers/infiniband/sw/rxe/rxe.h +++ b/drivers/infiniband/sw/rxe/rxe.h @@ -92,6 +92,8 @@ static inline u32 rxe_crc32(struct rxe_dev *rxe, return retval; } +extern bool rxe_init_done; + void rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu); int rxe_add(struct rxe_dev *rxe, unsigned int mtu); diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c index d5ed7571128f..bb54992e3ff2 100644 --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c @@ -77,6 +77,11 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp) struct net_device *ndev = NULL; struct rxe_dev *rxe; + if (!rxe_init_done) { + pr_err("add: module not initializes\n"); + err = -EINVAL; + goto err; + } len = sanitize_arg(val, intf, sizeof(intf)); if (!len) { pr_err("add: invalid interface name\n");
When using the module option 'add' during modprobe the module will crash as the option is evaluated before the module itself is initialized. So return an error when rxe_param_set_add() is called before the module is initialized. Signed-off-by: Hannes Reinecke <hare@suse.com> --- drivers/infiniband/sw/rxe/rxe.c | 6 ++++++ drivers/infiniband/sw/rxe/rxe.h | 2 ++ drivers/infiniband/sw/rxe/rxe_sysfs.c | 5 +++++ 3 files changed, 13 insertions(+)