Re: [PATCH] soc: samsung: exynos-pmu: Fix for CONFIG_DEBUG_PREEMPT

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/09/2025 12:15, Will Deacon wrote:
> Krzysztof,
> 
> On Sat, Sep 06, 2025 at 09:07:02AM +0200, Krzysztof Kozlowski wrote:
>> On 05/09/2025 19:43, Mostafa Saleh wrote:
>>>>>
>>>>> As this value is only read once, it doesn't require to be stable, so
>>>>
>>>> Why it does not need to be stable? Onlining wrong CPU number is not
>>>> desired...
>>>>
>>>>> just use "raw_smp_processor_id" instead.
>>>>
>>>> You might be just hiding some other real issue, because above stacktrace
>>>> is from gs101_cpuhp_pmu_online() which IRQs disabled and preemption
>>>> disabled. Provide analysis of the warning, instead of just making it
>>>> disappear.
>>>
>>> Not sure I understand, how is preemption disabled? that wouldn't fire
>>> in that case.
>>
>> Because there is explicit preempt_disable().
> 
> Where do you see that?

I did look at the code.

All the calls I saw (including calltrace from commit msg) were under raw
spinlock and raw spinlock does:

preempt_disable();

> 
> From what I can tell, the driver (somewhat bizarrely) registers its
> online callback much earlier (at CPUHP_BP_PREPARE_DYN) than the offline
> callback so that gs101_cpuhp_pmu_online() will be run in the context of
> the caller/onliner rather than the actual CPU coming up. I don't see
> anything which disables preemption on that path.
> 
>> So far you did not present any code analysis, any actual arguments, so
>> deflecting reviewer's comment like above will get you nowhere. Instead
>> of replying with a question, bring arguments that the code path does not
>> disable the preemption.
> 
> Mostafa's reported a bug and had a go at fixing it, for goodness sake.
> Would you rather not hear about bugs in the code you maintain?

I am happy to see such patch, but Mostafa instead of replying with
anything useful just shoved back my comment back to me.


> 
> As somebody who should understand this code better than most, perhaps
> you could try a bit harder to fill in the blanks on the technical side

I did and I investigated the code and I cannot find the issue leading to
it, thus I asked.

And then contributor just replied whatever so I will go away and stop
bothering them?

> rather than pointing out extraneous blank lines and making questionable
> judgements about CC lines.
> 
>> My call is that you run all this on some other kernel, just like usually
>> happens in Google.
> 
> Whilst that inevitably happens with some of the more product-facing
> teams, I think it's foolish to assume that nobody works directly with
> upstream at Google. We're also not going to waste time fabricating bug

I saw many, many  bug reports from Google and Linaro done on Android
kernel. Really, too many.

> reports which is why we bother to reproduce issues on Pixel 6, as that
> can boot upstream without any additional patches.

Best regards,
Krzysztof