On Sat, 2012-10-20 at 11:37 +0900, Chanwoo Choi wrote: > On 10/20/2012 11:30 AM, anish kumar wrote: > > On Sat, 2012-10-20 at 10:57 +0900, Chanwoo Choi wrote: > >> On 10/19/2012 02:12 AM, anish kumar wrote: > >>> From: anish kumar <anish198519851...@gmail.com> > >>> > >>> We don't need a release call in this file as we are doing > >>> everything needed in unregister call and we don't have any > >>> more pointer to free up. > >>> > >>> Signed-off-by: anish kumar <anish198519851...@gmail.com> > >>> --- > >>> drivers/extcon/extcon-class.c | 4 +--- > >>> 1 files changed, 1 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c > >>> index 946a318..cf30eb1 100644 > >>> --- a/drivers/extcon/extcon-class.c > >>> +++ b/drivers/extcon/extcon-class.c > >>> @@ -585,9 +585,7 @@ static void extcon_cleanup(struct extcon_dev *edev, > >>> bool skip) > >>> > >>> static void extcon_dev_release(struct device *dev) > >>> { > >>> - struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev); > >>> - > >>> - extcon_cleanup(edev, true); > >>> + /* We don't have any thing to free here */ > >>> } > >>> > >>> static const char *muex_name = "mutually_exclusive"; > >> > >> I can't agree this patch. The extcon_dev_release() function is used > >> for dev->release. If some case without calling extcon_dev_unregister(), > >> I think dev->release function is needed to free memory of edev->dev. > > Is it not being released by extcon_dev_unregister? > > I think it is released by that and we will do two times free and > > list_del(&edev->entry) as it is called by extcon_dev_release also. > > I think that this patch should modify it as below patch to remove > two call of kfree(). How about you? > > diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c > index e717bbc..efca0b4 100644 > --- a/drivers/extcon/extcon-class.c > +++ b/drivers/extcon/extcon-class.c > @@ -597,9 +597,8 @@ static void extcon_cleanup(struct extcon_dev *edev, bool > skip) > #endif > device_unregister(edev->dev); > put_device(edev->dev); > - } > - > - kfree(edev->dev); > + } else { > + kfree(edev->dev) How about below?
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c index 946a318..62e4ecb 100644 --- a/drivers/extcon/extcon-class.c +++ b/drivers/extcon/extcon-class.c @@ -551,43 +551,9 @@ static int create_extcon_class(void) return 0; } -static void extcon_cleanup(struct extcon_dev *edev, bool skip) -{ - mutex_lock(&extcon_dev_list_lock); - list_del(&edev->entry); - mutex_unlock(&extcon_dev_list_lock); - - if (!skip && get_device(edev->dev)) { - int index; - - if (edev->mutually_exclusive && edev->max_supported) { - for (index = 0; edev->mutually_exclusive[index]; - index++) - kfree(edev->d_attrs_muex[index].attr.name); - kfree(edev->d_attrs_muex); - kfree(edev->attrs_muex); - } - - for (index = 0; index < edev->max_supported; index++) - kfree(edev->cables[index].attr_g.name); - - if (edev->max_supported) { - kfree(edev->extcon_dev_type.groups); - kfree(edev->cables); - } - - device_unregister(edev->dev); - put_device(edev->dev); - } - - kfree(edev->dev); -} - static void extcon_dev_release(struct device *dev) { - struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev); - - extcon_cleanup(edev, true); + kfree(edev->dev); } static const char *muex_name = "mutually_exclusive"; @@ -813,7 +779,32 @@ EXPORT_SYMBOL_GPL(extcon_dev_register); */ void extcon_dev_unregister(struct extcon_dev *edev) { - extcon_cleanup(edev, false); + mutex_lock(&extcon_dev_list_lock); + list_del(&edev->entry); + mutex_unlock(&extcon_dev_list_lock); + + if (!skip && get_device(edev->dev)) { + int index; + + if (edev->mutually_exclusive && edev->max_supported) { + for (index = 0; edev->mutually_exclusive[index]; + index++) + kfree(edev->d_attrs_muex[index].attr.name); + kfree(edev->d_attrs_muex); + kfree(edev->attrs_muex); + } + + for (index = 0; index < edev->max_supported; index++) + kfree(edev->cables[index].attr_g.name); + + if (edev->max_supported) { + kfree(edev->extcon_dev_type.groups); + kfree(edev->cables); + } + + device_unregister(edev->dev); + put_device(edev->dev); + } } EXPORT_SYMBOL_GPL(extcon_dev_unregister); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/