lkml.org 
[lkml]   [2017]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] ipmi_si: use smi_num for init_name
From
Date
On 04/10/2017 01:43 PM, minyard@acm.org wrote:
> From: Tony Camuso <tcamuso@redhat.com>
>
> 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 <tcamuso@redhat.com>
>
> 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 <cminyard@mvista.com>
> ---
>
> 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]);
>
>

\
 
 \ /
  Last update: 2017-04-10 19:49    [W:0.039 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site