The exit() callback is optional and shouldn't be called without checking
a valid pointer first.
Also, we must clear freq_table pointer even if the exit() callback isn't
present.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/cpufreq.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 66e10a19d76a..fd9c3ed21f49 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1679,10 +1679,13 @@ static void __cpufreq_offline(unsigned int cpu, struct cpufreq_policy *policy)
*/
if (cpufreq_driver->offline) {
cpufreq_driver->offline(policy);
- } else if (cpufreq_driver->exit) {
- cpufreq_driver->exit(policy);
- policy->freq_table = NULL;
+ return;
}
+
+ if (cpufreq_driver->exit)
+ cpufreq_driver->exit(policy);
+
+ policy->freq_table = NULL;
}
static int cpufreq_offline(unsigned int cpu)
@@ -1740,7 +1743,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
}
/* We did light-weight exit earlier, do full tear down now */
- if (cpufreq_driver->offline)
+ if (cpufreq_driver->offline && cpufreq_driver->exit)
cpufreq_driver->exit(policy);
up_write(&policy->rwsem);
--
2.31.1.272.g89b43f80a514
On 12-04-24, 14:12, lizhe wrote:
> I was the first one to find this problem, so the patch should be submitted by me.
:)
This patch doesn't take away any of the work you have done. What you are trying
to do is simplify drivers with empty exit callback and the unused return value
of the callback.
And what I am trying to do is fix a bug in the cpufreq core, which only makes
your other patches more acceptable.
So no, you never identified the problem this patch is trying to solve.
Please don't feel that anyone is trying to take away your hardwork. That's not
how things are done here. We appreciate anyone who is spending time to make the
kernel better.
If I were to take credit of your work, then I would have sent a big patch to fix
the exit() callback issue you are trying to solve, with randomly sent patches.
--
viresh
On 12-04-24, 14:19, lizhe wrote:
> Why did you do that? Why did you plagiarize others' achievements? Is this the style of Linaro?
Even if your changes make sense, the discussions needs to be healthy. I am a
co-maintainer of the cpufreq subsystem and its mine and Rafael's responsibility
to keep things moving in the right direction.
This patch fixes a issue you never mentioned over LKML.
Lets not make this awkward, it won't help anyone.
Thanks.
--
viresh
Getting the Cc list back, + Greg.
Greg,
Looks like another one of those experiments with the community ?
:)
On 12-04-24, 14:27, lizhe wrote:
> You are really disgusting and have no manners at all. This makes people feel disgusted with your company.
>
>
>
> ---- Replied Message ----
> | From | Viresh Kumar<[email protected]> |
> | Date | 04/12/2024 14:24 |
> | To | lizhe<[email protected]> |
> | Cc | rafael<[email protected]>、linux-pm<[email protected]>、Vincent Guittot<[email protected]>、linux-kernel<[email protected]> |
> | Subject | Re: [PATCH] cpufreq: exit() callback is optional |
> On 12-04-24, 14:12, lizhe wrote:
> > I was the first one to find this problem, so the patch should be submitted by me.
>
> :)
>
> This patch doesn't take away any of the work you have done. What you are trying
> to do is simplify drivers with empty exit callback and the unused return value
> of the callback.
>
> And what I am trying to do is fix a bug in the cpufreq core, which only makes
> your other patches more acceptable.
>
> So no, you never identified the problem this patch is trying to solve.
>
> Please don't feel that anyone is trying to take away your hardwork. That's not
> how things are done here. We appreciate anyone who is spending time to make the
> kernel better.
>
> If I were to take credit of your work, then I would have sent a big patch to fix
> the exit() callback issue you are trying to solve, with randomly sent patches.
--
viresh
On Fri, Apr 12, 2024 at 7:49 AM Viresh Kumar <[email protected]> wrote:
>
> The exit() callback is optional and shouldn't be called without checking
> a valid pointer first.
>
> Also, we must clear freq_table pointer even if the exit() callback isn't
> present.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/cpufreq/cpufreq.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 66e10a19d76a..fd9c3ed21f49 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1679,10 +1679,13 @@ static void __cpufreq_offline(unsigned int cpu, struct cpufreq_policy *policy)
> */
> if (cpufreq_driver->offline) {
> cpufreq_driver->offline(policy);
> - } else if (cpufreq_driver->exit) {
> - cpufreq_driver->exit(policy);
> - policy->freq_table = NULL;
> + return;
> }
> +
> + if (cpufreq_driver->exit)
> + cpufreq_driver->exit(policy);
> +
> + policy->freq_table = NULL;
> }
>
> static int cpufreq_offline(unsigned int cpu)
> @@ -1740,7 +1743,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
> }
>
> /* We did light-weight exit earlier, do full tear down now */
> - if (cpufreq_driver->offline)
> + if (cpufreq_driver->offline && cpufreq_driver->exit)
> cpufreq_driver->exit(policy);
>
> up_write(&policy->rwsem);
> --
I have applied this patch (for 6.10 because I don't think it is
urgent) because it addresses both issues with missing ->exit() driver
callback checks. I honestly don't think it would be better to apply a
separate patch for each of them.
Thanks!