From patchwork Wed Dec 14 10:10:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chanwoo Choi X-Patchwork-Id: 9473981 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 63A12607EE for ; Wed, 14 Dec 2016 10:42:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 49B2128715 for ; Wed, 14 Dec 2016 10:42:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3DC6028718; Wed, 14 Dec 2016 10:42:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5DDDF28715 for ; Wed, 14 Dec 2016 10:42:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755220AbcLNKmC (ORCPT ); Wed, 14 Dec 2016 05:42:02 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:58543 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755185AbcLNKmB (ORCPT ); Wed, 14 Dec 2016 05:42:01 -0500 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Received: from epcpsbgm1new.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OI600JR16Y8YS40@mailout1.samsung.com> for linux-pm@vger.kernel.org; Wed, 14 Dec 2016 19:10:56 +0900 (KST) X-AuditID: cbfee61a-f79916d0000062de-cf-58511aaf98ac Received: from epmmp2 ( [203.254.227.17]) by epcpsbgm1new.samsung.com (EPCPMTA) with SMTP id 3E.8F.25310.FAA11585; Wed, 14 Dec 2016 19:10:56 +0900 (KST) Content-transfer-encoding: 8BIT Received: from [10.113.62.212] by mmp2.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OI600NGF6Y70Z60@mmp2.samsung.com>; Wed, 14 Dec 2016 19:10:55 +0900 (KST) Message-id: <58511AAF.7010705@samsung.com> Date: Wed, 14 Dec 2016 19:10:55 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Sudeep Holla , Chris Diamand , linux-pm@vger.kernel.org, MyungJoo Ham Cc: Kyungmin Park , Nishanth Menon , Cai Zhiyong , "cpgs (cpgs@samsung.com)" Subject: Re: [PATCH] PM / devfreq: Don't delete sysfs group twice References: <20161213170935.GA4661@e107465-lin.cambridge.arm.com> <5850A286.9080302@samsung.com> <5850B30A.20701@samsung.com> <420ba56f-70be-0735-ad99-d90dd04113c5@arm.com> In-reply-to: <420ba56f-70be-0735-ad99-d90dd04113c5@arm.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrIIsWRmVeSWpSXmKPExsVy+t9jQd0NUoERBr+fM1lc2befxWLap8ss Fi8PaVqcbXrDbvG59wijxe3GFWwWb36cZbJYfmoHiwOHx5p5axg9Wo68ZfXo27KK0eP4je1M Hp83yQWwRrnZZKQmpqQWKaTmJeenZOal2yqFhrjpWigp5CXmptoqRej6hgQpKZQl5pQCeUYG aMDBOcA9WEnfLsEto/nyOeaCXQoV8xb0MjcwnpbsYuTkkBAwkbi0/h4bhC0mceHeejBbSGAW o8TkHZUgNq+AoMSPyfdYuhg5OJgF5CWOXMqGMNUlpkzJ7WLkAqp+wChx68cRFohyLYklJ68w gdgsAqoSzR0LweJsQPH9L26AjecXUJS4+uMxI8gcUYEIie4TYJtEBCYzStxrYwSZySywgFGi a1cLI0hCWMBR4umCVYwQy9qYJKZ/Xg+W4BSwlji06j37BEbBWUhOnYVw6iyEUxcwMq9ilEgt SC4oTkrPNcxLLdcrTswtLs1L10vOz93ECI6+Z1I7GA/ucj/EKMDBqMTDu0AwIEKINbGsuDL3 EKMEB7OSCC+XaGCEEG9KYmVValF+fFFpTmrxIUZToF8nMkuJJucDE0NeSbyhibmJubGBhbml pYmRkjhv4+xn4UIC6YklqdmpqQWpRTB9TBycUg2MMlMWPVLbwPzCcRVf3OPdlR+yW/9kPIqy e2K64p9UvV2rdqtYmMlsQb21e/pnaK2s9NKdtypg65t75+50/HnIv+QPj9xOnorPIk82VBZY v18YON/24KbtGR7GjxPT7ku/iXj45JSNYoWJc/0q19lbpL6XnS3oP+CpbxBerG766ef6ynfT s9/sU2Ipzkg01GIuKk4EAG8UBo/UAgAA X-MTR: 20000000000000000@CPGS Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Sudeep, On 2016년 12월 14일 18:29, Sudeep Holla wrote: > > > On 14/12/16 02:48, Chanwoo Choi wrote: >> Hi Chris, >> >> On 2016년 12월 14일 10:38, Chanwoo Choi wrote: >>> Hi Chris, >>> >>> On 2016년 12월 14일 02:09, Chris Diamand wrote: >>>> The 'userspace' governor adds a sysfs entry, which is removed when >>>> the governor is changed, or the devfreq device is released. However, >>>> when the latter occurs via device_unregister(), device_del() is >>>> called first, which removes the sysfs entries recursively and deletes >>>> the kobject. >>>> >>>> This means we get an Oops when the governor calls >>>> sysfs_remove_group() on the deleted kobject. Fix this by explicitly >>>> stopping the governor in devfreq_remove_device(), *before* the >>>> devfreq entry is removed. >>>> >>>> Note that we can't just remove the call to sysfs_remove_group() in >>>> the userspace governor, as it's needed for when the governor is >>>> changed to one without a sysfs entry. >>>> >>>> Signed-off-by: Chris Diamand >>>> --- >>>> drivers/devfreq/devfreq.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>>> index 478006b..d8a817c 100644 >>>> --- a/drivers/devfreq/devfreq.c >>>> +++ b/drivers/devfreq/devfreq.c >>>> @@ -622,6 +622,12 @@ int devfreq_remove_device(struct devfreq *devfreq) >>>> if (!devfreq) >>>> return -EINVAL; >>>> >>>> + if (devfreq->governor) { >>>> + devfreq->governor->event_handler(devfreq, >>>> + DEVFREQ_GOV_STOP, NULL); >>>> + devfreq->governor = NULL; >>>> + } >>>> + >>>> >>> The devfreq_remove_device has following description: >>> - "* The oppsite of devfreq_add_device()" >>> >>> I think that we should call the '_remove_devfreq()' function in the devfreq_remove_device() >>> because '_remove_devfreq()' is in charge of releasing the devfreq instance. >>> >>> The '_remove_devfreq() ' function includes the code to stop the governor and following works: >>> - remove the devfreq instance from devfreq list >>> - call the profile->exit function >>> - destroy the muxte of devfreq instance >>> - Free the devfreq memory. >> >> How about following patch? >> I think that this patch covers the all of case when removing the devfreq device. >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index 8a456dd55a8d..eea406df0037 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -496,6 +496,7 @@ static void _remove_devfreq(struct devfreq *devfreq) >> devfreq->profile->exit(devfreq->dev.parent); >> >> mutex_destroy(&devfreq->lock); >> + device_unregister(&devfreq->dev); >> kfree(devfreq); >> } >> > > Won't this result in device_unregister->put_device->kobject_put->kref_put-> > kobject_release->kobject_cleanup->release->device_unregister > > Is that fine ? > My suggestion is incorrect. The original suggestion is right with removing the code related to DEVFREQ_GOV_STOP from _remove_devfreq(). I'm sorry for confusion. diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 8a456dd55a8d..3e9eb7fa9e72 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -488,10 +488,6 @@ static void _remove_devfreq(struct devfreq *devfreq) list_del(&devfreq->node); mutex_unlock(&devfreq_list_lock); - if (devfreq->governor) - devfreq->governor->event_handler(devfreq, - DEVFREQ_GOV_STOP, NULL); - if (devfreq->profile->exit) devfreq->profile->exit(devfreq->dev.parent); @@ -634,6 +630,10 @@ int devfreq_remove_device(struct devfreq *devfreq) if (!devfreq) return -EINVAL; + if (devfreq->governor) + devfreq->governor->event_handler(devfreq, + DEVFREQ_GOV_STOP, NULL); + device_unregister(&devfreq->dev); return 0;