On 04/10/2017 01:43 PM, [email protected] wrote: > From: Tony Camuso <[email protected]> > > Commit 1abf71e moved the creation of new_smi->dev to earlier in the init > sequence in order to provide infrastructure for log printing. > > However, the init_name was created with a hard-coded value of zero. This > presents a problem in systems with more than one interface, producing a > call trace in dmesg. > > To correct the problem, simply use smi_num instead of the hard-coded > value of zero. > > Tested on a lenovo x3950. > > Signed-off-by: Tony Camuso <[email protected]> > > There was actually a more general problem, the platform device wasn't > being set correctly, either, and there was a possible (though extremely > unlikely) race on smi_num. Add locks to clean up the race and use the > proper value for the platform device, too.
Ouch! I thought smi_num was already locked by smi_infos. Now I see that it wasn't. Thanks! Regards, tony > > Tested on qemu in various configurations. > > Signed-off-by: Corey Minyard <[email protected]> > --- > > I reworked this a bit, I think I've covered all the problems with > smi_num. Tony, does this work for you? > > Thanks > > drivers/char/ipmi/ipmi_si_intf.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > b/drivers/char/ipmi/ipmi_si_intf.c > index 2a7c425..b2b618f 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -1954,7 +1954,9 @@ static int hotmod_handler(const char *val, struct > kernel_param *kp) > kfree(info); > goto out; > } > + mutex_lock(&smi_infos_lock); > rv = try_smi_init(info); > + mutex_unlock(&smi_infos_lock); > if (rv) { > cleanup_one_si(info); > goto out; > @@ -2042,8 +2044,10 @@ static int hardcode_find_bmc(void) > info->slave_addr = slave_addrs[i]; > > if (!add_smi(info)) { > + mutex_lock(&smi_infos_lock); > if (try_smi_init(info)) > cleanup_one_si(info); > + mutex_unlock(&smi_infos_lock); > ret = 0; > } else { > kfree(info); > @@ -3492,6 +3496,11 @@ static int add_smi(struct smi_info *new_smi) > return rv; > } > > +/* > + * Try to start up an interface. Must be called with smi_infos_lock > + * held, primarily to keep smi_num consistent, we only one to do these > + * one at a time. > + */ > static int try_smi_init(struct smi_info *new_smi) > { > int rv = 0; > @@ -3524,9 +3533,12 @@ static int try_smi_init(struct smi_info *new_smi) > goto out_err; > } > > + new_smi->intf_num = smi_num; > + > /* Do this early so it's available for logs. */ > if (!new_smi->dev) { > - init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", 0); > + init_name = kasprintf(GFP_KERNEL, "ipmi_si.%d", > + new_smi->intf_num); > > /* > * If we don't already have a device from something > @@ -3593,8 +3605,6 @@ static int try_smi_init(struct smi_info *new_smi) > > new_smi->interrupt_disabled = true; > atomic_set(&new_smi->need_watch, 0); > - new_smi->intf_num = smi_num; > - smi_num++; > > rv = try_enable_event_buffer(new_smi); > if (rv == 0) > @@ -3661,6 +3671,9 @@ static int try_smi_init(struct smi_info *new_smi) > goto out_err_stop_timer; > } > > + /* Don't increment till we know we have succeeded. */ > + smi_num++; > + > dev_info(new_smi->dev, "IPMI %s interface initialized\n", > si_to_str[new_smi->si_type]); > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
