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
