From: MyungJoo Ham <myungjoo.ham@samsung.com>
To: Kant Fan <kant@allwinnertech.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"allwinner-opensource-support@allwinnertech.com"
<allwinner-opensource-support@allwinnertech.com>
Subject: RE: Re: [RESEND] devfreq: governor: Save void *data in the governor userspace
Date: Wed, 14 Sep 2022 18:43:55 +0900 [thread overview]
Message-ID: <20220914094355epcms1p8d6eeb0834e2d7c782d49558fac0ce6c4@epcms1p8> (raw)
In-Reply-To: <a040e16c-e8e9-f550-7251-99ec1d18f508@allwinnertech.com>
>On 3/29/2022 5:14 PM, Kant Fan wrote:
>> The member void *data in the structure devfreq can be overwrite
>> by governor_userspace.
>> Signed-off-by: Kant Fan <kant@allwinnertech.com>
>> ---
>> drivers/devfreq/governor_userspace.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>> index ab9db7adb3ad..dbbb448dcbcf 100644
>> --- a/drivers/devfreq/governor_userspace.c
>> +++ b/drivers/devfreq/governor_userspace.c
>> @@ -17,6 +17,7 @@
>> struct userspace_data {
>> unsigned long user_frequency;
>> bool valid;
>> + void *saved_data;
>> };
>>
>> static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>> @@ -91,6 +92,7 @@ static int userspace_init(struct devfreq *devfreq)
>> goto out;
>> }
>> data->valid = false;
>> + data->saved_data = devfreq->data;
>> devfreq->data = data;
>>
>> err = sysfs_create_group(&devfreq->dev.kobj, &dev_attr_group);
>> @@ -100,6 +102,8 @@ static int userspace_init(struct devfreq *devfreq)
>>
>> static void userspace_exit(struct devfreq *devfreq)
>> {
>> + struct userspace_data *data = devfreq->data;
>> + void *saved_data = data->saved_data;
>> /*
>> * Remove the sysfs entry, unless this is being called after
>> * device_del(), which should have done this already via kobject_del().
>> @@ -108,7 +112,7 @@ static void userspace_exit(struct devfreq *devfreq)
>> sysfs_remove_group(&devfreq->dev.kobj, &dev_attr_group);
>>
>> kfree(devfreq->data);
>> - devfreq->data = NULL;
>> + devfreq->data = saved_data;
>> }
>>
>> static int devfreq_userspace_handler(struct devfreq *devfreq,
>
>Dear MyungJoo, Kyungmin & Chanwoo,
>Gently ping this issue... Does this patch has a chance to be accepted?
>This seems to be a bug in devfreq userspace governor, which affects the
>switching between governors -- When switching from userspace to
>ondemand, the ondemand governor would be invalid.
>If there's any question, please let me know.
>Thank you.
Yes, indeed. This is a bug.
Actually, it appears that allocating a new memory buffer for
devfreq->data itself is a bug for a governor, this is supposed
to be allocated by a device driver. Thus, the comment of
"void *data" of "struct devfreq" should be updated:
"/* private data for governors given by device drivers */"
It'd be better to have something like, "void *internal_data"
for governors to freely handle within its context of init-exit,
which is not touched by its users (device drivers).
@Chanwoo: what's your opinion on this?
Cheers,
MyungJoo.
next prev parent reply other threads:[~2022-09-14 9:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-29 9:14 [RESEND] devfreq: governor: Save void *data in the governor userspace Kant Fan
2022-09-14 9:15 ` Kant Fan
[not found] ` <CGME20220914091525epcas1p31c8a9b11ba7a36da4678e3254b0ea962@epcms1p8>
2022-09-14 9:43 ` MyungJoo Ham [this message]
2022-09-15 7:41 ` Kant Fan
2022-09-21 13:15 ` Kant Fan
2022-09-26 9:50 ` MyungJoo Ham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220914094355epcms1p8d6eeb0834e2d7c782d49558fac0ce6c4@epcms1p8 \
--to=myungjoo.ham@samsung.com \
--cc=allwinner-opensource-support@allwinnertech.com \
--cc=cw00.choi@samsung.com \
--cc=kant@allwinnertech.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).