From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751949Ab0G0CGN (ORCPT ); Mon, 26 Jul 2010 22:06:13 -0400 Received: from vms173007pub.verizon.net ([206.46.173.7]:48920 "EHLO vms173007pub.verizon.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751496Ab0G0CGL (ORCPT ); Mon, 26 Jul 2010 22:06:11 -0400 Message-id: <4C4E3F03.4030707@acm.org> Date: Mon, 26 Jul 2010 21:05:55 -0500 From: Corey Minyard User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100423 Thunderbird/3.0.4 MIME-version: 1.0 To: Yinghai Lu Cc: Andrew Morton , Matthew Garrett , Len Brown , Myron Stowe , openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen References: <4C4E3575.3020203@kernel.org> In-reply-to: <4C4E3575.3020203@kernel.org> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Please run this through checkpatch, as it has coding style violations. Also, this patch appears to fix bugs in addition to adding the print. Can we have a separate patch for that? I'm also not clear on the reason for this. I believe all this information is already available in /proc/ipmi//params. I don't think there is a strong reason to print it to the log. -corey On 07/26/2010 08:25 PM, Yinghai Lu wrote: > also print out the reg spacing and size for spmi and smbios. > so bios guys could have idea to make them consistent. > > Signed-off-by: Yinghai Lu > > --- > drivers/char/ipmi/ipmi_si_intf.c | 47 ++++++++++++++++++++++++++++++++------- > 1 file changed, 39 insertions(+), 8 deletions(-) > > Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c > =================================================================== > --- linux-2.6.orig/drivers/char/ipmi/ipmi_si_intf.c > +++ linux-2.6/drivers/char/ipmi/ipmi_si_intf.c > @@ -1804,9 +1804,12 @@ static int hotmod_handler(const char *va > info->irq_setup = std_irq_setup; > info->slave_addr = ipmb; > > - if (!add_smi(info)) > + if (!add_smi(info)) { > if (try_smi_init(info)) > cleanup_one_si(info); > + } else { > + kfree(info); > + } > } else { > /* remove */ > struct smi_info *e, *tmp_e; > @@ -1890,9 +1893,12 @@ static __devinit void hardcode_find_bmc( > info->irq_setup = std_irq_setup; > info->slave_addr = slave_addrs[i]; > > - if (!add_smi(info)) > + if (!add_smi(info)) { > if (try_smi_init(info)) > cleanup_one_si(info); > + } else { > + kfree(info); > + } > } > } > > @@ -2088,7 +2094,13 @@ static __devinit int try_init_spmi(struc > } > info->io.addr_data = spmi->addr.address; > > - add_smi(info); > + pr_info("ipmi_si: SPMI: %s %#lx regsize %d spacing %d irq %d\n", > + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", > + info->io.addr_data, info->io.regsize, info->io.regspacing, > + info->irq); > + > + if (add_smi(info)) > + kfree(info); > > return 0; > } > @@ -2204,7 +2216,10 @@ static int __devinit ipmi_pnp_probe(stru > res, info->io.regsize, info->io.regspacing, > info->irq); > > - return add_smi(info); > + if (add_smi(info)) > + goto err_free; > + > + return 0; > > err_free: > kfree(info); > @@ -2362,7 +2377,13 @@ static __devinit void try_init_dmi(struc > if (info->irq) > info->irq_setup = std_irq_setup; > > - add_smi(info); > + pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n", > + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem", > + info->io.addr_data, info->io.regsize, info->io.regspacing, > + info->irq); > + > + if (add_smi(info)) > + kfree(info); > } > > static void __devinit dmi_find_bmc(void) > @@ -2468,7 +2489,10 @@ static int __devinit ipmi_pci_probe(stru > &pdev->resource[0], info->io.regsize, info->io.regspacing, > info->irq); > > - return add_smi(info); > + if (add_smi(info)) > + kfree(info); > + > + return 0; > } > > static void __devexit ipmi_pci_remove(struct pci_dev *pdev) > @@ -2581,7 +2605,12 @@ static int __devinit ipmi_of_probe(struc > > dev_set_drvdata(&dev->dev, info); > > - return add_smi(info); > + if (add_smi(info)) { > + kfree(info); > + return -EBUSY; > + } > + > + return 0; > } > > static int __devexit ipmi_of_remove(struct of_device *dev) > @@ -3018,6 +3047,8 @@ static __devinit void default_find_bmc(v > info->io.addr_data); > } else > cleanup_one_si(info); > + } else { > + kfree(info); > } > } > } > @@ -3045,7 +3076,7 @@ static int add_smi(struct smi_info *new_ > si_to_str[new_smi->si_type]); > mutex_lock(&smi_infos_lock); > if (!is_new_interface(new_smi)) { > - printk(KERN_CONT PFX "duplicate interface\n"); > + printk(KERN_CONT " duplicate interface\n"); > rv = -EBUSY; > goto out_err; > } >