* [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen
@ 2010-07-27 1:25 Yinghai Lu
2010-07-27 2:05 ` Corey Minyard
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yinghai Lu @ 2010-07-27 1:25 UTC (permalink / raw)
To: Corey Minyard, Andrew Morton, Matthew Garrett, Len Brown,
Myron Stowe
Cc: openipmi-developer, linux-kernel
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 <yinghai@kernel.org>
---
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;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen
2010-07-27 1:25 [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen Yinghai Lu
@ 2010-07-27 2:05 ` Corey Minyard
2010-07-27 4:41 ` Yinghai Lu
2010-07-27 4:57 ` [PATCH 1/2] " Yinghai Lu
2010-07-27 4:58 ` [PATCH 2/2] ipmi: print info for spmi and smbios path like acpi and pci Yinghai Lu
2 siblings, 1 reply; 7+ messages in thread
From: Corey Minyard @ 2010-07-27 2:05 UTC (permalink / raw)
To: Yinghai Lu
Cc: Andrew Morton, Matthew Garrett, Len Brown, Myron Stowe,
openipmi-developer, linux-kernel
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/<if#>/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<yinghai@kernel.org>
>
> ---
> 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;
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen
2010-07-27 2:05 ` Corey Minyard
@ 2010-07-27 4:41 ` Yinghai Lu
2010-07-27 14:42 ` Corey Minyard
0 siblings, 1 reply; 7+ messages in thread
From: Yinghai Lu @ 2010-07-27 4:41 UTC (permalink / raw)
To: Corey Minyard
Cc: Andrew Morton, Matthew Garrett, Len Brown, Myron Stowe,
openipmi-developer, linux-kernel
On 07/26/2010 07:05 PM, Corey Minyard wrote:
> Please run this through checkpatch, as it has coding style violations.
yhlu@linux-siqj:~/xx/xx/kernel/tip/linux-2.6> ./scripts/checkpatch.pl patches/ipmi_reg_size.patch
total: 0 errors, 0 warnings, 105 lines checked
patches/ipmi_reg_size.patch has no obvious style problems and is ready for submission.
yhlu@linux-siqj:~/xx/xx/kernel/tip/linux-2.6>
>
> Also, this patch appears to fix bugs in addition to adding the print.
> Can we have a separate patch for that?
in the comment log, i already mentioned that.
will separate it to twol
>
> I'm also not clear on the reason for this. I believe all this
> information is already available in /proc/ipmi/<if#>/params. I don't
> think there is a strong reason to print it to the log.
then why there is printing for ACPI path and pci path?
Yinghai
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] ipmi: Fix memleaking for add_smi when duplicating happen
2010-07-27 1:25 [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen Yinghai Lu
2010-07-27 2:05 ` Corey Minyard
@ 2010-07-27 4:57 ` Yinghai Lu
2010-07-27 15:44 ` Myron Stowe
2010-07-27 4:58 ` [PATCH 2/2] ipmi: print info for spmi and smbios path like acpi and pci Yinghai Lu
2 siblings, 1 reply; 7+ messages in thread
From: Yinghai Lu @ 2010-07-27 4:57 UTC (permalink / raw)
To: Corey Minyard, Andrew Morton, Matthew Garrett, Len Brown,
Myron Stowe
Cc: openipmi-developer, linux-kernel
need free the temp info struct when we have duplicated ones
-v2: seperate printing change to another patch
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/char/ipmi/ipmi_si_intf.c | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 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,8 @@ static __devinit int try_init_spmi(struc
}
info->io.addr_data = spmi->addr.address;
- add_smi(info);
+ if (add_smi(info))
+ kfree(info);
return 0;
}
@@ -2204,7 +2211,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 +2372,8 @@ static __devinit void try_init_dmi(struc
if (info->irq)
info->irq_setup = std_irq_setup;
- add_smi(info);
+ if (add_smi(info))
+ kfree(info);
}
static void __devinit dmi_find_bmc(void)
@@ -2468,7 +2479,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 +2595,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 +3037,8 @@ static __devinit void default_find_bmc(v
info->io.addr_data);
} else
cleanup_one_si(info);
+ } else {
+ kfree(info);
}
}
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ipmi: print info for spmi and smbios path like acpi and pci
2010-07-27 1:25 [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen Yinghai Lu
2010-07-27 2:05 ` Corey Minyard
2010-07-27 4:57 ` [PATCH 1/2] " Yinghai Lu
@ 2010-07-27 4:58 ` Yinghai Lu
2 siblings, 0 replies; 7+ messages in thread
From: Yinghai Lu @ 2010-07-27 4:58 UTC (permalink / raw)
To: Corey Minyard, Andrew Morton, Matthew Garrett, Len Brown,
Myron Stowe
Cc: openipmi-developer, linux-kernel
print out the reg spacing and size for spmi and smbios.
so bios guys could have idea to make them consistent.
also remove extra PFX on duplicating path.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/char/ipmi/ipmi_si_intf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
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
@@ -2094,6 +2094,11 @@ static __devinit int try_init_spmi(struc
}
info->io.addr_data = spmi->addr.address;
+ 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);
@@ -2372,6 +2377,11 @@ static __devinit void try_init_dmi(struc
if (info->irq)
info->irq_setup = std_irq_setup;
+ 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);
}
@@ -3066,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;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen
2010-07-27 4:41 ` Yinghai Lu
@ 2010-07-27 14:42 ` Corey Minyard
0 siblings, 0 replies; 7+ messages in thread
From: Corey Minyard @ 2010-07-27 14:42 UTC (permalink / raw)
To: Yinghai Lu
Cc: Andrew Morton, Matthew Garrett, Len Brown, Myron Stowe,
openipmi-developer, linux-kernel
On 07/26/2010 11:41 PM, Yinghai Lu wrote:
> On 07/26/2010 07:05 PM, Corey Minyard wrote:
>
>> Please run this through checkpatch, as it has coding style violations.
>>
> yhlu@linux-siqj:~/xx/xx/kernel/tip/linux-2.6> ./scripts/checkpatch.pl patches/ipmi_reg_size.patch
> total: 0 errors, 0 warnings, 105 lines checked
>
> patches/ipmi_reg_size.patch has no obvious style problems and is ready for submission.
> yhlu@linux-siqj:~/xx/xx/kernel/tip/linux-2.6>
>
Sorry, you are right.
>
>> Also, this patch appears to fix bugs in addition to adding the print.
>> Can we have a separate patch for that?
>>
> in the comment log, i already mentioned that.
>
> will separate it to twol
>
Ok, thanks
>
>> I'm also not clear on the reason for this. I believe all this
>> information is already available in /proc/ipmi/<if#>/params. I don't
>> think there is a strong reason to print it to the log.
>>
> then why there is printing for ACPI path and pci path?
>
Well, I'm not sure on this. You are right, it is printed for those
paths and not for DMI or SPMI cases. Printing too much information is
not generally a good idea, but this may be useful. I guess to make it
consistent it would be best to add this.
-corey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ipmi: Fix memleaking for add_smi when duplicating happen
2010-07-27 4:57 ` [PATCH 1/2] " Yinghai Lu
@ 2010-07-27 15:44 ` Myron Stowe
0 siblings, 0 replies; 7+ messages in thread
From: Myron Stowe @ 2010-07-27 15:44 UTC (permalink / raw)
To: Yinghai Lu
Cc: Corey Minyard, Andrew Morton, Matthew Garrett, Len Brown,
openipmi-developer, linux-kernel
On Mon, 2010-07-26 at 21:57 -0700, Yinghai Lu wrote:
> need free the temp info struct when we have duplicated ones
Nice catch on these. Could you improve the patch description please.
Myron
>
> -v2: seperate printing change to another patch
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> drivers/char/ipmi/ipmi_si_intf.c | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 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,8 @@ static __devinit int try_init_spmi(struc
> }
> info->io.addr_data = spmi->addr.address;
>
> - add_smi(info);
> + if (add_smi(info))
> + kfree(info);
>
> return 0;
> }
> @@ -2204,7 +2211,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 +2372,8 @@ static __devinit void try_init_dmi(struc
> if (info->irq)
> info->irq_setup = std_irq_setup;
>
> - add_smi(info);
> + if (add_smi(info))
> + kfree(info);
> }
>
> static void __devinit dmi_find_bmc(void)
> @@ -2468,7 +2479,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 +2595,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 +3037,8 @@ static __devinit void default_find_bmc(v
> info->io.addr_data);
> } else
> cleanup_one_si(info);
> + } else {
> + kfree(info);
> }
> }
> }
>
--
Myron Stowe HP Open Source Linux Lab (OSLL)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-07-27 15:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-27 1:25 [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen Yinghai Lu
2010-07-27 2:05 ` Corey Minyard
2010-07-27 4:41 ` Yinghai Lu
2010-07-27 14:42 ` Corey Minyard
2010-07-27 4:57 ` [PATCH 1/2] " Yinghai Lu
2010-07-27 15:44 ` Myron Stowe
2010-07-27 4:58 ` [PATCH 2/2] ipmi: print info for spmi and smbios path like acpi and pci Yinghai Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox