From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754025AbdDJRnz (ORCPT ); Mon, 10 Apr 2017 13:43:55 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:33151 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751585AbdDJRnw (ORCPT ); Mon, 10 Apr 2017 13:43:52 -0400 From: minyard@acm.org To: Tony Camuso Cc: openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org, Corey Minyard Subject: [PATCH] ipmi_si: use smi_num for init_name Date: Mon, 10 Apr 2017 12:43:37 -0500 Message-Id: <1491846217-384-1-git-send-email-minyard@acm.org> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Tony Camuso 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 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. Tested on qemu in various configurations. Signed-off-by: Corey Minyard --- 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]); -- 2.7.4