From: Michael Kelley <mhklinux@outlook.com>
To: Saurabh Singh Sengar <ssengar@linux.microsoft.com>
Cc: "kys@microsoft.com" <kys@microsoft.com>,
"haiyangz@microsoft.com" <haiyangz@microsoft.com>,
"wei.liu@kernel.org" <wei.liu@kernel.org>,
"decui@microsoft.com" <decui@microsoft.com>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"ssengar@microsoft.com" <ssengar@microsoft.com>
Subject: RE: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V
Date: Mon, 19 Feb 2024 17:53:25 +0000 [thread overview]
Message-ID: <SN6PR02MB41573212FCDBDF58305881ECD4512@SN6PR02MB4157.namprd02.prod.outlook.com> (raw)
In-Reply-To: <20240218071727.GA19702@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Saturday, February 17, 2024 11:17 PM
> > >
> > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > index 0024210..bc3f496 100644
> > > --- a/drivers/hv/Kconfig
> > > +++ b/drivers/hv/Kconfig
> > > @@ -9,6 +9,7 @@ config HYPERV
> > > select PARAVIRT
> > > select X86_HV_CALLBACK_VECTOR if X86
> > > select OF_EARLY_FLATTREE if OF
> > > + select CPUMASK_OFFSTACK
> > > help
> > > Select this option to run Linux as a Hyper-V client operating
> > > system.
> > > --
> > > 1.8.3.1
> > >
> >
> > I'm not sure that enabling CPUMASK_OFFSTACK for Hyper-V
> > guests is the right thing to do, as there's additional runtime
> > cost when CPUMASK_OFFSTACK is enabled. I agree that for
> > the most general case, you want NR_CPUS to be 2048, which
> > requires CPUMASK_OFFSTACK. But it would be legitimate to
> > build a kernel with NR_CPUS set to something like 64 or 256
> > for a more limited Hyper-V guest use case, and to not want to
> > incur the cost of CPUMASK_OFFSTACK.
> >
> > You could consider doing something like this:
> >
> > select CPUMASK_OFFSTACK if NR_CPUS > 512
>
> Thanks for your review.
>
> This was my first thought as well, but for x86, NR_CPUS itself depends
> on CPUMASK_OFFSTACK and this creates some kind of circular dependency
> and doesn't work effectively.
>
> Here are few key points to note:
>
> 1. In ARM64 as well for enabling CPUMASK_OFFSTACK we need to enable
> DEBUG_PER_CPU_MAPS and that will have additional overhead.
> This dependency is for all the archs. There was an earlier attempt
> to decouple it: https://lore.kernel.org/lkml/20220412231508.32629-1-libo.chen@oracle.com/
>
> 2. However, for ARM64, NR_CPUS doesn't have dependency on CPUMASK_OFFSTACK.
> In ARM64 NR_CPUS is quite independent from any policy, we can choose any
> value for NR_CPUS freely, things are simple. This problem specificaly
> to be solved for x86.
>
> 3. If we have to select more then 512 CPUs on x86, CPUMASK_OFFSTACK
> needto be enabled, so this additional runtime cost is unavoidable
> for NR_CPUS > 512. There is no way today to enable CPUMASK_OFFSTACK
> apart from enabling MAXSMP or DEBUG_PER_CPU_MAPS. Both of these
> options we don't want to use.
>
> I agree that we possibly don't want to enable this option for HyperV VMs
> where NR_CPUS < 512. I have two thoughts here:
>
> 1. Enable it only for VTL platforms, as current requirement for minimal kernel
> is only for VTL platforms only.
>
> 2. Fix this for all of x86. I couldn't find any reson why CPUMASK_OFFSTACK
> dependency is there on x86 for having more than 512 CPUs. What is special
> in x86 to have this restriction ? If there is no reason we should relax
> the restriction of CPUMASK_OFFSTACK for NR_CPUs similar to ARM and other
> archs.
>
You've done some deeper research than I did. :-( What a mess.
ARM64 seems to have it right. On x86, the dependency between NR_CPUS
and CPUMASK_OFFSTACK seems to flow the wrong direction. I would think
you would select NR_CPUS first, and then if the number is large, select
CPUMASK_OFFSTACK.
And the display of CPUMASK_OFFSTACK in config tools should not be
dependent on DEBUG_PER_CPU_MAPS. It should be easy to independently
select CPUMASK_OFFSTACK (modulo architectures that don't support it).
In the Libo Chen thread, I don't understand the reluctance to make
CPUMASK_OFFSTACK independent of DEBUG_PER_CPU_MAPS.
I don't have any great suggestions for the path forward. :-( Maybe
revive the Libo Chen thread, with a better justification for removing
the dependency between CPUMASK_OFFSTACK and
DEBUG_PER_CPU_MAPS? Or at least clarify why the dependency
should be kept?
Michael
prev parent reply other threads:[~2024-02-19 17:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-16 14:10 [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V Saurabh Sengar
2024-02-17 16:46 ` Michael Kelley
2024-02-18 7:17 ` Saurabh Singh Sengar
2024-02-19 17:53 ` Michael Kelley [this message]
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=SN6PR02MB41573212FCDBDF58305881ECD4512@SN6PR02MB4157.namprd02.prod.outlook.com \
--to=mhklinux@outlook.com \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=kys@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ssengar@linux.microsoft.com \
--cc=ssengar@microsoft.com \
--cc=wei.liu@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 read-only IMAP folder(s) and NNTP newsgroup(s).