[PATCH] cpufreq: exit() callback is optional

[PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

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 <viresh.kumar@linaro.org>
---
 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

Re: [PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

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

Re: [PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

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<viresh.kumar@linaro.org> |
> | Date | 04/12/2024 14:24 |
> | To | lizhe<sensor1010@163.com> |
> | Cc | rafael<rafael@kernel.org>、linux-pm<linux-pm@vger.kernel.org>、Vincent Guittot<vincent.guittot@linaro.org>、linux-kernel<linux-kernel@vger.kernel.org> |
> | 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

Re: [PATCH] cpufreq: exit() callback is optional

[Greg KH]

On Fri, Apr 12, 2024 at 02:46:25PM +0800, lizhe wrote:
> Look at what you've done, it really makes people feel extremely disgusted with your company. Your company will receive a very bad reputation.

I think you need to stop now, this is not doing anything productive at
all and is not acceptable.

greg k-h

Re: [PATCH] cpufreq: exit() callback is optional

[Greg KH]

On Fri, Apr 12, 2024 at 05:05:05PM +0800, lizhe wrote:
> I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.

lore.kernel.org links for this please?

Re: [PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

On 12-04-24, 11:14, Greg KH wrote:
> On Fri, Apr 12, 2024 at 05:05:05PM +0800, lizhe wrote:
> > I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.
> 
> lore.kernel.org links for this please?

Yeah, I just posted those in a separate reply. There was some
confusion in the beginning and I already acked the concerned patch few
hours back. Rafael may apply them later.

Not sure why the emails are still coming despite that..

-- 
viresh

Re: [PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

On 12-04-24, 17:05, lizhe wrote:
> I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.

Well, I decided not to reply to your emails anymore but this needs to
be clarified a bit now.

You sent a lot of patches, over and over again and it was a mess. I
saw the this [1] series first and went over to read the code and fixed
an issue which I found (by the $subject patch).

Later I read your other patch [2], which I Acked roughly two hours
back and yes you did send a patch that fixed the problem partially. I
never saw it earlier, which is okay and it happens. Despite me giving
an Ack to your patch, you have sent half-a-dozen more emails..

Then I posted a newer version of my patch some time back, removing the
bits you already fixed [3].

That is all one side of the story. But all the noise you have created
here has really demotivated people to review your stuff now.

--
Viresh

[1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/
[2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/
[3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/

Re: [PATCH] cpufreq: exit() callback is optional

[Greg KH]

On Fri, Apr 12, 2024 at 02:51:08PM +0530, Viresh Kumar wrote:
> On 12-04-24, 17:05, lizhe wrote:
> > I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.
> 
> Well, I decided not to reply to your emails anymore but this needs to
> be clarified a bit now.
> 
> You sent a lot of patches, over and over again and it was a mess. I
> saw the this [1] series first and went over to read the code and fixed
> an issue which I found (by the $subject patch).
> 
> Later I read your other patch [2], which I Acked roughly two hours
> back and yes you did send a patch that fixed the problem partially. I
> never saw it earlier, which is okay and it happens. Despite me giving
> an Ack to your patch, you have sent half-a-dozen more emails..
> 
> Then I posted a newer version of my patch some time back, removing the
> bits you already fixed [3].
> 
> That is all one side of the story. But all the noise you have created
> here has really demotivated people to review your stuff now.
> 
> --
> Viresh
> 
> [1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/
> [2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/
> [3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/

Thanks for the links, I don't see that you did anything wrong here at
all.

Lizhe, you seem to be confused as to how kernel development works.  I
suggest you take some time off and read up on how this all is supposed
to happen and then work with some local people, in person, to get this
figured out first, before submitting changes again.

thanks,

greg k-h

Re:Re:Re: [PATCH] cpufreq: exit() callback is optional

[lizhe]

[-- Attachment #1.1.1: Type: text/plain, Size: 2358 bytes --]

HI


i will review how it all started.
1. 
  i  reported the vulnerability to the maintainer.
   


2. manitaner replay me


3.   i report to main line 
     


4.  you reply me 
     


5 .  you report to main line 
      




You submitted the patch to the mainline after you learned about the vulnerability from the patch I submitted




    



At 2024-04-12 21:54:00, "lizhe" <sensor1010@163.com> wrote:




Hi 

     Viresh Kumar
     please add my name to the signature. because i discovered this vulnerability. 
     please reply me.
     thanks




   

 


At 2024-04-12 18:28:56, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>On Fri, Apr 12, 2024 at 02:51:08PM +0530, Viresh Kumar wrote:
>> On 12-04-24, 17:05, lizhe wrote:
>> > I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.
>> 
>> Well, I decided not to reply to your emails anymore but this needs to
>> be clarified a bit now.
>> 
>> You sent a lot of patches, over and over again and it was a mess. I
>> saw the this [1] series first and went over to read the code and fixed
>> an issue which I found (by the $subject patch).
>> 
>> Later I read your other patch [2], which I Acked roughly two hours
>> back and yes you did send a patch that fixed the problem partially. I
>> never saw it earlier, which is okay and it happens. Despite me giving
>> an Ack to your patch, you have sent half-a-dozen more emails..
>> 
>> Then I posted a newer version of my patch some time back, removing the
>> bits you already fixed [3].
>> 
>> That is all one side of the story. But all the noise you have created
>> here has really demotivated people to review your stuff now.
>> 
>> --
>> Viresh
>> 
>> [1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/
>> [2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/
>> [3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/
>
>Thanks for the links, I don't see that you did anything wrong here at
>all.
>
>Lizhe, you seem to be confused as to how kernel development works.  I
>suggest you take some time off and read up on how this all is supposed
>to happen and then work with some local people, in person, to get this
>figured out first, before submitting changes again.
>
>thanks,
>
>greg k-h

[-- Attachment #1.1.2: Type: text/html, Size: 5554 bytes --]

[-- Attachment #1.2: 01.png --]
[-- Type: image/png, Size: 348854 bytes --]

[-- Attachment #1.3: 08.png --]
[-- Type: image/png, Size: 306234 bytes --]

[-- Attachment #1.4: 06.png --]
[-- Type: image/png, Size: 132311 bytes --]

[-- Attachment #1.5: 07.png --]
[-- Type: image/png, Size: 170643 bytes --]

[-- Attachment #1.6: 09.png --]
[-- Type: image/png, Size: 132699 bytes --]

[-- Attachment #2: 01.png --]
[-- Type: image/png, Size: 348854 bytes --]

Re:Re:Re: [PATCH] cpufreq: exit() callback is optional

[lizhe]

[-- Attachment #1.1.1: Type: text/plain, Size: 2494 bytes --]



Let's both take a step back and add my signature to the patch,
since I was the one who discovered and reported the vulnerability.




HI


i will review how it all started.
1. 
  i  reported the vulnerability to the maintainer.
   


2. manitaner replay me


3.   i report to main line 
     


4.  you reply me 
     


5 .  you report to main line 
      




You submitted the patch to the mainline after you learned about the vulnerability from the patch I submitted




    



At 2024-04-12 21:54:00, "lizhe" <sensor1010@163.com> wrote:




Hi 

     Viresh Kumar
     please add my name to the signature. because i discovered this vulnerability. 
     please reply me.
     thanks




   

 


At 2024-04-12 18:28:56, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>On Fri, Apr 12, 2024 at 02:51:08PM +0530, Viresh Kumar wrote:
>> On 12-04-24, 17:05, lizhe wrote:
>> > I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.
>> 
>> Well, I decided not to reply to your emails anymore but this needs to
>> be clarified a bit now.
>> 
>> You sent a lot of patches, over and over again and it was a mess. I
>> saw the this [1] series first and went over to read the code and fixed
>> an issue which I found (by the $subject patch).
>> 
>> Later I read your other patch [2], which I Acked roughly two hours
>> back and yes you did send a patch that fixed the problem partially. I
>> never saw it earlier, which is okay and it happens. Despite me giving
>> an Ack to your patch, you have sent half-a-dozen more emails..
>> 
>> Then I posted a newer version of my patch some time back, removing the
>> bits you already fixed [3].
>> 
>> That is all one side of the story. But all the noise you have created
>> here has really demotivated people to review your stuff now.
>> 
>> --
>> Viresh
>> 
>> [1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/
>> [2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/
>> [3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/
>
>Thanks for the links, I don't see that you did anything wrong here at
>all.
>
>Lizhe, you seem to be confused as to how kernel development works.  I
>suggest you take some time off and read up on how this all is supposed
>to happen and then work with some local people, in person, to get this
>figured out first, before submitting changes again.
>
>thanks,
>
>greg k-h

[-- Attachment #1.1.2: Type: text/html, Size: 6379 bytes --]

[-- Attachment #1.2: 01.png --]
[-- Type: image/png, Size: 348854 bytes --]

[-- Attachment #1.3: 08.png --]
[-- Type: image/png, Size: 306234 bytes --]

[-- Attachment #1.4: 06.png --]
[-- Type: image/png, Size: 132311 bytes --]

[-- Attachment #1.5: 07.png --]
[-- Type: image/png, Size: 170643 bytes --]

[-- Attachment #1.6: 09.png --]
[-- Type: image/png, Size: 132699 bytes --]

[-- Attachment #2: 01.png --]
[-- Type: image/png, Size: 348854 bytes --]

Re: Re:Re:Re: [PATCH] cpufreq: exit() callback is optional

[shuah]

Hi Lizhe,

On 4/12/24 08:41, lizhe wrote:
> 
> Let's both take a step back and add my signature to the patch,
> since I was the one who discovered and reported the vulnerability.
> 
> 

You might have discovered the vulnerability and sent in a patch. After that,
it is totally up to the maintainer to decide to accept or reject the patch.
Developers can't demand their patches to be reviewed and/or accepted. They
can request a review and inclusion if maintainer deems it acceptable.

In this email thread, I can see that maintainers and developers have been
advising you to understand the kernel development process.

Refer to the following document to understand the process:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst

Refer to the following Contributor Covenant Code of Conduct to understand the
code of conduct the Linux kernel community abides by:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst

thanks,
-- Shuah (On behalf of the Code of Conduct Committee)

Re: [PATCH] cpufreq: exit() callback is optional

[Shuah Khan]

On 4/22/24 05:33, lizhe wrote:
> The maintainer's obvious robbery behavior.
> I submitted patches to the main branch. After receiving them, the maintainer submitted patches again, and the patches contained the patches I submitted.
> That is to say, the patches submitted by the maintainer included mine. It is the maintainer who failed to follow the rules.
> What's wrong with me submitting patches to the main branch according to the rules!
> 

This is the last email I will be sending to you. It doesn't appear
you are willing to engage with the kernel community in a productive
and constructive manner.

> Refer to the following document to understand the process:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst>
> 
> Refer to the following Contributor Covenant Code of Conduct to understand the
> code of conduct the Linux kernel community abides by:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst>
> 

-- thanks,
Shuah (On behalf of the Code of Conduct Committee)

Re: [PATCH] cpufreq: exit() callback is optional

[Viresh Kumar]

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

Re: [PATCH] cpufreq: exit() callback is optional

[Rafael J. Wysocki]

On Fri, Apr 12, 2024 at 7:49 AM Viresh Kumar <viresh.kumar@linaro.org> 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 <viresh.kumar@linaro.org>
> ---
>  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!