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
Date: Wed, 12 Jan 2022 17:59:28 +0000 [thread overview]
Message-ID: <Yd8XAI7G4sEab9Nh@google.com> (raw)
In-Reply-To: <20220111033629.GC2175@gao-cwp>
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.
next prev parent reply other threads:[~2022-01-12 17:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-27 8:15 [PATCH 0/6] Improve KVM's interaction with CPU hotplug Chao Gao
2021-12-27 8:15 ` [PATCH 1/6] KVM: x86: Move check_processor_compatibility from init ops to runtime ops Chao Gao
2022-01-10 23:27 ` Sean Christopherson
2022-01-11 3:36 ` Chao Gao
2022-01-12 17:59 ` Sean Christopherson [this message]
2021-12-27 8:15 ` [PATCH 2/6] KVM: x86: Use kvm_x86_ops in kvm_arch_check_processor_compat Chao Gao
2022-01-10 21:10 ` Sean Christopherson
2022-01-11 3:06 ` Chao Gao
2021-12-27 8:15 ` [PATCH 3/6] KVM: Remove opaque from kvm_arch_check_processor_compat Chao Gao
2022-01-10 23:06 ` Sean Christopherson
2022-01-11 3:19 ` Chao Gao
2022-01-12 17:20 ` Sean Christopherson
2022-01-12 17:21 ` Sean Christopherson
2021-12-27 8:15 ` [PATCH 4/6] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section Chao Gao
2021-12-27 8:15 ` [PATCH 5/6] KVM: x86: Remove WARN_ON in kvm_arch_check_processor_compat Chao Gao
2022-01-10 22:59 ` Sean Christopherson
2022-01-11 2:15 ` Tian, Kevin
2022-01-11 19:48 ` Sean Christopherson
2022-01-12 11:00 ` Chao Gao
2022-01-12 17:35 ` Sean Christopherson
2022-01-17 13:35 ` Chao Gao
2022-01-17 13:46 ` Chao Gao
2022-01-19 0:34 ` Sean Christopherson
2021-12-27 8:15 ` [PATCH 6/6] KVM: Do compatibility checks on hotplugged CPUs Chao Gao
2022-01-11 0:46 ` Sean Christopherson
2022-01-11 5:32 ` Chao Gao
2022-01-12 17:52 ` Sean Christopherson
2022-01-12 23:01 ` Jim Mattson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yd8XAI7G4sEab9Nh@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=chao.gao@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).