[PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

[PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

[Saurabh Sengar]

CPUMASK_OFFSTACK must be set to have NR_CPUS_RANGE_END value greater than
512, which eventually allows NR_CPUS > 512.

CPUMASK_OFFSTACK can also be enabled by setting MAXSMP=y, but that will
set NR_CPUS=8192. This is not accurate for Hyper-V, because maximum number
of vCPU supported by Hyper-V today is 2048. Thus, enabling MAXSMP increase
the vmlinux size unnecessary.

This option allows NR_CPUS=2048 which saves around 1MB of vmlinux size for
Hyper-V.

Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
 drivers/hv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

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

RE: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

[Michael Kelley]

From: Saurabh Sengar <ssengar@linux.microsoft.com> Sent: Friday, February 16, 2024 6:10 AM
> To: kys@microsoft.com; haiyangz@microsoft.com; wei.liu@kernel.org;
> decui@microsoft.com; linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: ssengar@microsoft.com
> Subject: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V
> 
> CPUMASK_OFFSTACK must be set to have NR_CPUS_RANGE_END value greater than
> 512, which eventually allows NR_CPUS > 512.
> 
> CPUMASK_OFFSTACK can also be enabled by setting MAXSMP=y, but that will
> set NR_CPUS=8192. This is not accurate for Hyper-V, because maximum number
> of vCPU supported by Hyper-V today is 2048. Thus, enabling MAXSMP increase
> the vmlinux size unnecessary.

Note that these statements apply only to x86.  arm64 doesn't have MAXSMP
or NR_CPUS_RANGE_END.

> 
> This option allows NR_CPUS=2048 which saves around 1MB of vmlinux size
> for Hyper-V.
> 
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
>  drivers/hv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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

But kernel builders always have the option of explicitly
enabling CPUMASK_OFFSTACK.  That's what I see in the distro
vendor arm64 images in Azure, since there's currently nothing
that automatically selects CPUMASK_OFFSTACK for arm64.
So I'm wondering if selecting CPUMASK_OFFSTACK under
HYPERV should be added at all.  The two aren't really related.

There are recent LKML threads on enabling CPUMASK_OFFSTACK
for arm64 -- see links below for some useful discussion of the
topic in general.

Michael

[1] https://lore.kernel.org/lkml/794a1211-630b-3ee5-55a3-c06f10df1490@linux.com/
[2] https://lore.kernel.org/lkml/7ab6660e-e69f-a64b-0de3-b8dde14f79fa@linux.com/
[3] https://lore.kernel.org/lkml/e0d41efb-a74e-6bb5-f325-63d42358c802@gentwo.org/

Re: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

[Saurabh Singh Sengar]

On Sat, Feb 17, 2024 at 04:46:04PM +0000, Michael Kelley wrote:
> From: Saurabh Sengar <ssengar@linux.microsoft.com> Sent: Friday, February 16, 2024 6:10 AM
> > To: kys@microsoft.com; haiyangz@microsoft.com; wei.liu@kernel.org;
> > decui@microsoft.com; linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: ssengar@microsoft.com
> > Subject: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V
> > 
> > CPUMASK_OFFSTACK must be set to have NR_CPUS_RANGE_END value greater than
> > 512, which eventually allows NR_CPUS > 512.
> > 
> > CPUMASK_OFFSTACK can also be enabled by setting MAXSMP=y, but that will
> > set NR_CPUS=8192. This is not accurate for Hyper-V, because maximum number
> > of vCPU supported by Hyper-V today is 2048. Thus, enabling MAXSMP increase
> > the vmlinux size unnecessary.
> 
> Note that these statements apply only to x86.  arm64 doesn't have MAXSMP
> or NR_CPUS_RANGE_END.
> 
> > 
> > This option allows NR_CPUS=2048 which saves around 1MB of vmlinux size
> > for Hyper-V.
> > 
> > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > ---
> >  drivers/hv/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > 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-2-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.

- Saurabh

> 
> But kernel builders always have the option of explicitly
> enabling CPUMASK_OFFSTACK.  That's what I see in the distro
> vendor arm64 images in Azure, since there's currently nothing
> that automatically selects CPUMASK_OFFSTACK for arm64.
> So I'm wondering if selecting CPUMASK_OFFSTACK under
> HYPERV should be added at all.  The two aren't really related.
> 
> There are recent LKML threads on enabling CPUMASK_OFFSTACK
> for arm64 -- see links below for some useful discussion of the
> topic in general.
> 
> Michael
> 
> [1] https://lore.kernel.org/lkml/794a1211-630b-3ee5-55a3-c06f10df1490@linux.com/
> [2] https://lore.kernel.org/lkml/7ab6660e-e69f-a64b-0de3-b8dde14f79fa@linux.com/
> [3] https://lore.kernel.org/lkml/e0d41efb-a74e-6bb5-f325-63d42358c802@gentwo.org/

RE: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

[Michael Kelley]

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