Date:   Wed, 12 Jan 2022 17:59:28 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Chao Gao <chao.gao@intel.com>
Cc:     kvm@vger.kernel.org, pbonzini@redhat.com, kevin.tian@intel.com,
        tglx@linutronix.de, Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, Ingo Molnar <mingo@redhat.com>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] KVM: x86: Move check_processor_compatibility from
 init ops to runtime ops
Message-ID: <Yd8XAI7G4sEab9Nh@google.com>
References: <20211227081515.2088920-1-chao.gao@intel.com>
 <20211227081515.2088920-2-chao.gao@intel.com>
 <YdzAzT5AqO0aCsHk@google.com>
 <20220111033629.GC2175@gao-cwp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220111033629.GC2175@gao-cwp>
Precedence: bulk

On Tue, Jan 11, 2022, Chao Gao wrote:
> On Mon, Jan 10, 2022 at 11:27:09PM +0000, Sean Christopherson wrote:
> >On Mon, Dec 27, 2021, Chao Gao wrote:
> >> so that KVM can do compatibility checks on hotplugged CPUs. Drop __init
> >> from check_processor_compatibility() and its callees.
> >
> >Losing the __init annotation on all these helpers makes me a bit sad, more from a
> >documentation perspective than a "but we could shave a few bytes" perspective.
> 
> This makes sense.
> 
> >More than once I've wondered why some bit of code isn't __init, only to realize
> >its used for hotplug.
> 
> Same problem to some global data structures which can be __initdata if hotplug
> isn't supported.
> 
> >
> >What if we added an __init_or_hotplug annotation that is a nop if HOTPLUG_CPU=y?
> 
> Personally __init_or_hotplug is a little long as an annotation. How about
> __hotplug?
> 
> One concern is: is it acceptable to introduce a new annotation and use it in
> new code but not fix all places that should use it in existing code.
> 
> I think the right process is
> 1. introduce a new annotation
> 2. fix existing code to use this annotation
> 3. add new code.
> 
> There is no doubt that #2 would take great effort. I'm not sure if it is really
> worth it.
> 
> >At a glance, KVM could use that if the guts of kvm_online_cpu() were #idef'd out
> >on !CONFIG_HOTPLUG_CPU.  That also give us a bit of test coverage for bots that
> >build with SMP=n.
> 
> Will do with your suggested-by.

I don't think you should try to add a new annotation in this series.  My question
really was just that, a question to others if there would be value in adding an
annotation to identify symbols that are !__init only because of hotplug.  Such new
functionality is certainly not required for fixing KVM's mishandling of hotplug,
and trying to cram it in here will bloat and slow down this series.