Messages in this thread | | | Date | Thu, 2 Aug 2012 07:57:27 -0700 | From | Guenter Roeck <> | Subject | Re: [PATCH] spi: omap2-mcspi: Fix the error handling in probe |
| |
On Thu, Aug 02, 2012 at 03:37:13PM +0530, Shubhrajyoti wrote: > On Wednesday 01 August 2012 08:37 PM, Guenter Roeck wrote: > > On Wed, Aug 01, 2012 at 03:06:28PM +0530, Shubhrajyoti D wrote: > >> The kfree() is taken care of by the spi core (spi_master_release() function) > >> that is called once the last reference to the underlying struct device has > >> been released. So the driver need not call kfree. > >> > >> Also the put was missed in some of the error handling fix the same. > >> There by fixing the missing device_put in some of the error paths. > >> > >> Cc: Guenter Roeck <linux@roeck-us.net> > > Reported-by: may be better here. > My bad. I should have done. > > > >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > Acked-by: Guenter Roeck <linux@roeck-us.net> > thanks. > > I suspect that "spi_master_put(master);" may also be missing in > > omap2_mcspi_remove(), but we'll need someone to confirm that. > Looks unlikely. > > spi_master_put does a > ... > if (master) > put_device(&master->dev); > ... > > In remove I call > > spi_unregister_master > ... > */ > void spi_unregister_master(struct spi_master *master) > { > int dummy; > [...] > > dummy = device_for_each_child(&master->dev, NULL, __unregister); > device_unregister(&master->dev); > } > > and > > void device_unregister(struct device *dev) > { > [..] > device_del(dev); > put_device(dev); > } > > Hope my understanding is correct. > I think it is; I checked the refcount. spi_register_master increases refcount from 1 to 3, and spi_unregister_master decreases it from 3 to 0.
Now, if _my_ understanding is correct, that means the data structure allocated with spi_alloc_master, and specifically the device private data structure (struct omap2_mcspi in your case), is freed with spi_unregister_master(). If so, it must not be accessed after the call to spi_unregister_master(). However, many drivers do access this data after the call to spi_unregister_master(). spi-tegra.c is a good example, but there are many others. Does that mean that those drivers access freed memory ?
Also, some other drivers do call spi_master_put() after spi_unregister_master(), with no matching spi_master_get() (eg spi-topcliff-pch.c). Does that mean that those drivers call spi_master_put() on free memory ?
Thanks, Guenter
| |