| Message ID | b97964653d02225f061e0c2a650b365c354b98c8.1712900945.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | cpufreq: exit() callback is optional | expand |
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.
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.
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.
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
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?
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/
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..
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
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!
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
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
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)
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);
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(-)