Re: [PATCH 00/14] paravirt: cleanup and reorg

Jürgen Groß
2025-09-11T08:00:13Z

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

On 11.09.25 09:48, Peter Zijlstra wrote:
> On Thu, Sep 11, 2025 at 08:34:19AM +0200, Juergen Gross wrote:
>> Some cleanups and reorg of paravirt code and headers:
>>
>> - The first 2 patches should be not controversial at all, as they
>> remove just some no longer needed #include and struct forward
>> declarations.
>>
>> - The 3rd patch is removing CONFIG_PARAVIRT_DEBUG, which IMO has
>> no real value, as it just changes a crash to a BUG() (the stack
>> trace will basically be the same). As the maintainer of the main
>> paravirt user (Xen) I have never seen this crash/BUG() to happen.
>>
>> - The 4th patch is just a movement of code.
>>
>> - I don't know for what reason asm/paravirt_api_clock.h was added,
>> as all archs supporting it do it exactly in the same way. Patch
>> 5 is removing it.
>>
>> - Patches 6-12 are streamlining the paravirt clock interfaces by
>> using a common implementation across architectures where possible
>> and by moving the related code into common sched code, as this is
>> where it should live.
>>
>> - Patches 13+14 are more like RFC material: patch 13 is doing some
>> preparation work to enable patch 14 to move all spinlock related
>> paravirt functions into qspinlock.h. If this approach is accepted,
>> I'd like to continue with this work by moving most (or all?)
>> paravirt functions from paravirt.h into the headers where their
>> native counterparts are defined. This is meant to keep the native
>> and paravirt function definitions together in one place and
>> hopefully to be able to reduce the include hell with paravirt.
>>
>> Juergen Gross (14):
>> x86/paravirt: remove not needed includes of paravirt.h
>> x86/paravirt: remove some unneeded struct declarations
>> x86/paravirt: remove PARAVIRT_DEBUG config option
>> x86/paravirt: move thunk macros to paravirt_types.h
>> paravirt: remove asm/paravirt_api_clock.h
>> sched: move clock related paravirt code to kernel/sched
>> arm/paravirt: use common code for paravirt_steal_clock()
>> arm64/paravirt: use common code for paravirt_steal_clock()
>> loongarch/paravirt: use common code for paravirt_steal_clock()
>> riscv/paravirt: use common code for paravirt_steal_clock()
>> x86/paravirt: use common code for paravirt_steal_clock()
>> x86/paravirt: move paravirt_sched_clock() related code into tsc.c
>> x86/paravirt: allow pv-calls outside paravirt.h
>> x86/pvlocks: move paravirt spinlock functions into qspinlock.h
>
> With the note that tip typically likes a capital after the prefix, like:
>
> x86/paravirt: Remove unneeded includes of paravirt.h

Noted, thanks.

>
> For 1-12:
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
>
> Now, as to the last two, I'm not sure. Leaking those macros out of PV
> isn't particularly nice, then again, not the end of the world either.
> Just not sure.

Yes, that's why I didn't continue with all of the other potential movement of
paravirt functions. I want some feedback first. :-)

Its a tradeoff between having functions with / without paravirt in one file
against hiding the paravirt stuff from "normal" readers (not writers, as those
probably need to touch the paravirt variant, too).

BTW, I think the macro leaking isn't the main problem. There are other macros
leaking already.

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]