Message-ID: <eb760054-1679-408a-9de9-2505f88382dd@suse.com>
Date: Thu, 11 Sep 2025 10:00:04 +0200
From: Jürgen Groß <jgross@...e.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
 linux-hyperv@...r.kernel.org, virtualization@...ts.linux.dev,
 loongarch@...ts.linux.dev, linuxppc-dev@...ts.ozlabs.org,
 linux-riscv@...ts.infradead.org, kvm@...r.kernel.org,
 Andy Lutomirski <luto@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
 "K. Y. Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, Will Deacon <will@...nel.org>,
 Boqun Feng <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>,
 Jiri Kosina <jikos@...nel.org>, Josh Poimboeuf <jpoimboe@...nel.org>,
 Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
 Boris Ostrovsky <boris.ostrovsky@...cle.com>,
 xen-devel@...ts.xenproject.org, Ajay Kaher <ajay.kaher@...adcom.com>,
 Alexey Makhalov <alexey.makhalov@...adcom.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>, Russell King
 <linux@...linux.org.uk>, Catalin Marinas <catalin.marinas@....com>,
 Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Alexandre Ghiti <alex@...ti.fr>, Juri Lelli <juri.lelli@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
 linux-arm-kernel@...ts.infradead.org, Paolo Bonzini <pbonzini@...hat.com>,
 Vitaly Kuznetsov <vkuznets@...hat.com>,
 Stefano Stabellini <sstabellini@...nel.org>,
 Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
 Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: Re: [PATCH 00/14] paravirt: cleanup and reorg

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@...radead.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

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3684 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)

Powered by blists - more mailing lists