| Message ID | 1421444828-32616-1-git-send-email-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Kalle Valo |
| Headers | show |
On 01/16/2015 10:47 PM, Rafa? Mi?ecki wrote: > Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> > --- > This code was introduced by Saul in > ee91592711ed90a1abfbb1b2ceadded11d685164 > bcma: don't leak memory for PCIE, MIPS, GBIT cores > > I don't really see reason for making it in so complicated way. > I tested my patch for crashes, but didn't really try kmemleak. > Is my simple solution OK? Or am I missing something? Anyone? Are you sure no device driver accesses the chipcommon, pcie or mips core in the unregister part? If some device driver does something in his remove function with e.g. chipcommon it will cause problems when the chipcommon core was already freed. Hauke > --- > drivers/bcma/main.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c > index c3c5e0a..58dd582 100644 > --- a/drivers/bcma/main.c > +++ b/drivers/bcma/main.c > @@ -371,6 +371,8 @@ static void bcma_unregister_cores(struct bcma_bus *bus) > list_del(&core->list); > if (core->dev_registered) > device_unregister(&core->dev); > + else > + kfree(core); > } > if (bus->hosttype == BCMA_HOSTTYPE_SOC) > platform_device_unregister(bus->drv_cc.watchdog); > @@ -467,7 +469,6 @@ int bcma_bus_register(struct bcma_bus *bus) > > void bcma_bus_unregister(struct bcma_bus *bus) > { > - struct bcma_device *cores[3]; > int err; > > err = bcma_gpio_unregister(&bus->drv_cc); > @@ -478,15 +479,7 @@ void bcma_bus_unregister(struct bcma_bus *bus) > > bcma_core_chipcommon_b_free(&bus->drv_cc_b); > > - cores[0] = bcma_find_core(bus, BCMA_CORE_MIPS_74K); > - cores[1] = bcma_find_core(bus, BCMA_CORE_PCIE); > - cores[2] = bcma_find_core(bus, BCMA_CORE_4706_MAC_GBIT_COMMON); > - > bcma_unregister_cores(bus); > - > - kfree(cores[2]); > - kfree(cores[1]); > - kfree(cores[0]); > } > > /* > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 January 2015 at 19:26, Hauke Mehrtens <hauke@hauke-m.de> wrote: > On 01/16/2015 10:47 PM, Rafa? Mi?ecki wrote: >> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> >> --- >> This code was introduced by Saul in >> ee91592711ed90a1abfbb1b2ceadded11d685164 >> bcma: don't leak memory for PCIE, MIPS, GBIT cores >> >> I don't really see reason for making it in so complicated way. >> I tested my patch for crashes, but didn't really try kmemleak. >> Is my simple solution OK? Or am I missing something? Anyone? > > Are you sure no device driver accesses the chipcommon, pcie or mips core > in the unregister part? If some device driver does something in his > remove function with e.g. chipcommon it will cause problems when the > chipcommon core was already freed. It didn't crash anything for me, but your point is correct. I'll change the logic to first unregister and then free cores.
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c index c3c5e0a..58dd582 100644 --- a/drivers/bcma/main.c +++ b/drivers/bcma/main.c @@ -371,6 +371,8 @@ static void bcma_unregister_cores(struct bcma_bus *bus) list_del(&core->list); if (core->dev_registered) device_unregister(&core->dev); + else + kfree(core); } if (bus->hosttype == BCMA_HOSTTYPE_SOC) platform_device_unregister(bus->drv_cc.watchdog); @@ -467,7 +469,6 @@ int bcma_bus_register(struct bcma_bus *bus) void bcma_bus_unregister(struct bcma_bus *bus) { - struct bcma_device *cores[3]; int err; err = bcma_gpio_unregister(&bus->drv_cc); @@ -478,15 +479,7 @@ void bcma_bus_unregister(struct bcma_bus *bus) bcma_core_chipcommon_b_free(&bus->drv_cc_b); - cores[0] = bcma_find_core(bus, BCMA_CORE_MIPS_74K); - cores[1] = bcma_find_core(bus, BCMA_CORE_PCIE); - cores[2] = bcma_find_core(bus, BCMA_CORE_4706_MAC_GBIT_COMMON); - bcma_unregister_cores(bus); - - kfree(cores[2]); - kfree(cores[1]); - kfree(cores[0]); } /*
Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> --- This code was introduced by Saul in ee91592711ed90a1abfbb1b2ceadded11d685164 bcma: don't leak memory for PCIE, MIPS, GBIT cores I don't really see reason for making it in so complicated way. I tested my patch for crashes, but didn't really try kmemleak. Is my simple solution OK? Or am I missing something? Anyone? --- drivers/bcma/main.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)