linux-kernel - Re: [PATCH] soc: samsung: exynos-pmu: Fix for CONFIG_DEBUG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <84332e77-cfd2-4090-a3c0-114a9eb5422a@kernel.org>
Date: Sat, 6 Sep 2025 09:07:02 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Mostafa Saleh <smostafa@...gle.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org, alim.akhtar@...sung.com,
 Peter Griffin <peter.griffin@...aro.org>,
 André Draszik <andre.draszik@...aro.org>
Subject: Re: [PATCH] soc: samsung: exynos-pmu: Fix for CONFIG_DEBUG_PREEMPT

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().

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.

My call is that you run all this on some other kernel, just like usually
happens in Google.

> 
> I am not familiar with this driver, but as I see the value is used
> only once, it would be stable, for example using get/put_cpu won't
> really matter, because next access doesn't depend on the current CPU.
> Otherwise, if you imply that this might not be enough, that means
> the driver is already broken even without CONFIG_DEBUG_PREEMP
> (which is beyond my knowledge at this point)
> 
>>
>>>
>>> Cc: Peter Griffin <peter.griffin@...aro.org>
>>> Cc: André Draszik <andre.draszik@...aro.org>
>>>
>>
>> No blank lines between tags.
>>
>> Missing Fixes tag... and then Cc is not necessary. Please follow
>> standard kernel process.
> 
> I will add the Fixes.
> 
> I am working with Peter and André, so I thought Cc is fine
> (according to the process) in:
> https://docs.kernel.org/process/5.Posting.html

Please learn how Fixes and get_maintainers.pl work.



Best regards,
Krzysztof