* [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations
@ 2023-10-18 16:37 Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (5 more replies)
0 siblings, 6 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Valentin Schneider, Srikar Dronamraju, Peter Zijlstra,
Rohan McLure, Nicholas Piggin, linuxppc-dev
PowerVM systems configured in shared processors mode have some unique
challenges. Some device-tree properties will be missing on a shared
processor. Hence some sched domains may not make sense for shared processor
systems.
Most shared processor systems are over-provisioned. Underlying PowerVM
Hypervisor would schedule at a Big Core granularity. The most recent power
processors support two almost independent cores. In a lightly loaded
condition, it helps the overall system performance if we pack to lesser
number of Big Cores.
System Configuration
type=Shared mode=Uncapped smt=8 lcpu=96 mem=1066409344 kB cpus=96 ent=64.00
So *64 Entitled cores/ 96 Virtual processor* Scenario
lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 768
On-line CPU(s) list: 0-767
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 16
Socket(s): 6
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 6 MiB (192 instances)
L1i cache: 9 MiB (192 instances)
NUMA node(s): 6
NUMA node0 CPU(s): 0-7,32-39,80-87,128-135,176-183,224-231,272-279,320-327,368-375,416-423,464-471,512-519,560-567,608-615,656-663,704-711,752-759
NUMA node1 CPU(s): 8-15,40-47,88-95,136-143,184-191,232-239,280-287,328-335,376-383,424-431,472-479,520-527,568-575,616-623,664-671,712-719,760-767
NUMA node4 CPU(s): 64-71,112-119,160-167,208-215,256-263,304-311,352-359,400-407,448-455,496-503,544-551,592-599,640-647,688-695,736-743
NUMA node5 CPU(s): 16-23,48-55,96-103,144-151,192-199,240-247,288-295,336-343,384-391,432-439,480-487,528-535,576-583,624-631,672-679,720-727
NUMA node6 CPU(s): 72-79,120-127,168-175,216-223,264-271,312-319,360-367,408-415,456-463,504-511,552-559,600-607,648-655,696-703,744-751
NUMA node7 CPU(s): 24-31,56-63,104-111,152-159,200-207,248-255,296-303,344-351,392-399,440-447,488-495,536-543,584-591,632-639,680-687,728-735
ebizzy -t 32 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 3840178 4059268 3978042 3973936.6 84264.456
+patch 5 3768393 3927901 3874994 3854046 71532.926 -3.01692
From lparstat (when the workload stabilized)
Kernel %user %sys %wait %idle physc %entc lbusy app vcsw phint
6.6.0-rc3 4.16 0.00 0.00 95.84 26.06 40.72 4.16 69.88 276906989 578
+patch 4.16 0.00 0.00 95.83 17.70 27.66 4.17 78.26 70436663 119
ebizzy -t 128 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 5520692 5981856 5717709 5727053.2 176093.2
+patch 5 5305888 6259610 5854590 5843311 375917.03 2.02998
From lparstat (when the workload stabilized)
Kernel %user %sys %wait %idle physc %entc lbusy app vcsw phint
6.6.0-rc3 16.66 0.00 0.00 83.33 45.49 71.08 16.67 50.50 288778533 581
+patch 16.65 0.00 0.00 83.35 30.15 47.11 16.65 65.76 85196150 133
ebizzy -t 512 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 19563921 20049955 19701510 19728733 198295.18
+patch 5 19455992 20176445 19718427 19832017 304094.05 0.523521
From lparstat (when the workload stabilized)
%Kernel user %sys %wait %idle physc %entc lbusy app vcsw phint
66.6.0-rc3 6.44 0.01 0.00 33.55 94.14 147.09 66.45 1.33 313345175 621
6+patch 6.44 0.01 0.00 33.55 94.15 147.11 66.45 1.33 109193889 309
System Configuration
type=Shared mode=Uncapped smt=8 lcpu=40 mem=1067539392 kB cpus=96 ent=40.00
So *40 Entitled cores/ 40 Virtual processor* Scenario
lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 320
On-line CPU(s) list: 0-319
Model name: POWER10 (architected), altivec supported
Model: 2.0 (pvr 0080 0200)
Thread(s) per core: 8
Core(s) per socket: 10
Socket(s): 4
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 2.5 MiB (80 instances)
L1i cache: 3.8 MiB (80 instances)
NUMA node(s): 4
NUMA node0 CPU(s): 0-7,32-39,64-71,96-103,128-135,160-167,192-199,224-231,256-263,288-295
NUMA node1 CPU(s): 8-15,40-47,72-79,104-111,136-143,168-175,200-207,232-239,264-271,296-303
NUMA node4 CPU(s): 16-23,48-55,80-87,112-119,144-151,176-183,208-215,240-247,272-279,304-311
NUMA node5 CPU(s): 24-31,56-63,88-95,120-127,152-159,184-191,216-223,248-255,280-287,312-319
ebizzy -t 32 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 3535518 3864532 3745967 3704233.2 130216.76
+patch 5 3608385 3708026 3649379 3651596.6 37862.163 -1.42099
%Kernel user %sys %wait %idle physc %entc lbusy app vcsw phint
6.6.0-rc3 10.00 0.01 0.00 89.99 22.98 57.45 10.01 41.01 1135139 262
+patch 10.00 0.00 0.00 90.00 16.95 42.37 10.00 47.05 925561 19
ebizzy -t 64 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 4434984 4957281 4548786 4591298.2 211770.2
+patch 5 4461115 4835167 4544716 4607795.8 151474.85 0.359323
%Kernel user %sys %wait %idle physc %entc lbusy app vcsw phint
6.6.0-rc3 20.01 0.00 0.00 79.99 38.22 95.55 20.01 25.77 1287553 265
+patch 19.99 0.00 0.00 80.01 25.55 63.88 19.99 38.44 1077341 20
ebizzy -t 256 -S 200 (5 iterations) Records per second. (Higher is better)
Kernel N Min Max Median Avg Stddev %Change
6.6.0-rc3 5 8850648 8982659 8951911 8936869.2 52278.031
+patch 5 8751038 9060510 8981409 8942268.4 117070.6 0.0604149
%Kernel user %sys %wait %idle physc %entc lbusy app vcsw phint
6.6.0-rc3 80.02 0.01 0.01 19.96 40.00 100.00 80.03 24.00 1597665 276
+patch 80.02 0.01 0.01 19.96 40.00 100.00 80.03 23.99 1383921 63
Observation:
We are able to see Improvement in ebizzy throughput even with lesser
core utilization (almost half the core utilization) in low utilization
scenarios while still retaining throughput in mid and higher utilization
scenarios.
Note: The numbers are with Uncapped + no-noise case. In the Capped and/or
noise case, due to contention on the Cores, the numbers are expected to
further improve.
Note: The numbers included (powerpc/paravirt: Improve vcpu_is_preempted)
https://lore.kernel.org/all/20231018155838.2332822-1-srikar@linux.vnet.ibm.com/
and (sched/fair: Enable group_asym_packing in find_idlest_group)
https://lore.kernel.org/all/20231018155036.2314342-1-srikar@linux.vnet.ibm.com/
Changelog
v1 (https://lore.kernel.org/all/20230830105244.62477-1-srikar@linux.vnet.ibm.com) -> v2:
1. Last two patches were added in this version
2. This version uses static keys
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Rohan McLure <rmclure@linux.ibm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Srikar Dronamraju (6):
powerpc/smp: Cache CPU has Asymmetric SMP
powerpc/smp: Enable Asym packing for cores on shared processor
powerpc/smp: Move shared_processor static key to smp.h
powerpc/smp: Disable MC domain for shared processor
powerpc/smp: Add read_mostly attribute
powerpc/smp: Avoid asym packing within thread_group of a core
arch/powerpc/include/asm/paravirt.h | 12 ------
arch/powerpc/include/asm/smp.h | 14 +++++++
arch/powerpc/kernel/smp.c | 62 +++++++++++++++++++++++------
3 files changed, 63 insertions(+), 25 deletions(-)
base-commit: eddc90ea2af5933249ea1a78119f2c8ef8d07156
--
2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, ndesaulniers, Mark Rutland,
linux-kernel
Currently cpu feature flag is checked whenever powerpc_smt_flags gets
called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
on the processor and all processors will either have this set or will
have it unset.
Hence only check for the feature flag once and cache it to be used
subsequently. This commit will help avoid a branch in powerpc_smt_flags
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog:
v1->v2: Using static keys instead of a variable.
Using pr_info_once instead of printk
arch/powerpc/kernel/smp.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5826f5108a12..37c41297c9ce 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -988,18 +988,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
}
static bool shared_caches;
+DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
#ifdef CONFIG_SCHED_SMT
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
- if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
- printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
- flags |= SD_ASYM_PACKING;
- }
- return flags;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
}
#endif
@@ -1686,6 +1684,11 @@ static void __init fixup_topology(void)
{
int i;
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
+ pr_info_once("Enabling Asymmetric SMT scheduling\n");
+ static_branch_enable(&powerpc_asym_packing);
+ }
+
#ifdef CONFIG_SCHED_SMT
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, ndesaulniers, linux-kernel, Rohan McLure,
Nicholas Piggin, linuxppc-dev, Josh Poimboeuf
Currently cpu feature flag is checked whenever powerpc_smt_flags gets
called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
on the processor and all processors will either have this set or will
have it unset.
Hence only check for the feature flag once and cache it to be used
subsequently. This commit will help avoid a branch in powerpc_smt_flags
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog:
v1->v2: Using static keys instead of a variable.
Using pr_info_once instead of printk
arch/powerpc/kernel/smp.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5826f5108a12..37c41297c9ce 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -988,18 +988,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
}
static bool shared_caches;
+DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
#ifdef CONFIG_SCHED_SMT
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
- if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
- printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
- flags |= SD_ASYM_PACKING;
- }
- return flags;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
}
#endif
@@ -1686,6 +1684,11 @@ static void __init fixup_topology(void)
{
int i;
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
+ pr_info_once("Enabling Asymmetric SMT scheduling\n");
+ static_branch_enable(&powerpc_asym_packing);
+ }
+
#ifdef CONFIG_SCHED_SMT
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, Mark Rutland, linux-kernel
If there are shared processor LPARs, underlying Hypervisor can have more
virtual cores to handle than actual physical cores.
Starting with Power 9, a core has 2 nearly independent thread groups.
On a shared processors LPARs, it helps to pack threads to lesser number
of cores so that the overall system performance and utilization
improves. PowerVM schedules at a core level. Hence packing to fewer
cores helps.
For example: Lets says there are two 8-core Shared LPARs that are
actually sharing a 8 Core shared physical pool, each running 8 threads
each. Then Consolidating 8 threads to 4 cores on each LPAR would help
them to perform better. This is because each of the LPAR will get
100% time to run applications and there will no switching required by
the Hypervisor.
To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog:
v1->v2: Using static key instead of a variable.
arch/powerpc/kernel/smp.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 37c41297c9ce..498c2d51fc20 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
*/
static int powerpc_shared_cache_flags(void)
{
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
+
return SD_SHARE_PKG_RESOURCES;
}
+static int powerpc_shared_proc_flags(void)
+{
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_ASYM_PACKING;
+
+ return 0;
+}
+
/*
* We can't just pass cpu_l2_cache_mask() directly because
* returns a non-const pointer and the compiler barfs on that.
@@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
#endif
{ shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
- { cpu_mc_mask, SD_INIT_NAME(MC) },
- { cpu_cpu_mask, SD_INIT_NAME(DIE) },
+ { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
+ { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
{ NULL, },
};
@@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
pr_info_once("Enabling Asymmetric SMT scheduling\n");
static_branch_enable(&powerpc_asym_packing);
+ } else if (is_shared_processor() && has_big_cores) {
+ static_branch_enable(&powerpc_asym_packing);
}
#ifdef CONFIG_SCHED_SMT
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, linux-kernel, Rohan McLure, Nicholas Piggin,
linuxppc-dev, Josh Poimboeuf
If there are shared processor LPARs, underlying Hypervisor can have more
virtual cores to handle than actual physical cores.
Starting with Power 9, a core has 2 nearly independent thread groups.
On a shared processors LPARs, it helps to pack threads to lesser number
of cores so that the overall system performance and utilization
improves. PowerVM schedules at a core level. Hence packing to fewer
cores helps.
For example: Lets says there are two 8-core Shared LPARs that are
actually sharing a 8 Core shared physical pool, each running 8 threads
each. Then Consolidating 8 threads to 4 cores on each LPAR would help
them to perform better. This is because each of the LPAR will get
100% time to run applications and there will no switching required by
the Hypervisor.
To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog:
v1->v2: Using static key instead of a variable.
arch/powerpc/kernel/smp.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 37c41297c9ce..498c2d51fc20 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
*/
static int powerpc_shared_cache_flags(void)
{
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
+
return SD_SHARE_PKG_RESOURCES;
}
+static int powerpc_shared_proc_flags(void)
+{
+ if (static_branch_unlikely(&powerpc_asym_packing))
+ return SD_ASYM_PACKING;
+
+ return 0;
+}
+
/*
* We can't just pass cpu_l2_cache_mask() directly because
* returns a non-const pointer and the compiler barfs on that.
@@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
{ cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
#endif
{ shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
- { cpu_mc_mask, SD_INIT_NAME(MC) },
- { cpu_cpu_mask, SD_INIT_NAME(DIE) },
+ { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
+ { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
{ NULL, },
};
@@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
pr_info_once("Enabling Asymmetric SMT scheduling\n");
static_branch_enable(&powerpc_asym_packing);
+ } else if (is_shared_processor() && has_big_cores) {
+ static_branch_enable(&powerpc_asym_packing);
}
#ifdef CONFIG_SCHED_SMT
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Ajay Kaher, Alexey Makhalov,
VMware PV-Drivers Reviewers, Josh Poimboeuf, virtualization, x86,
linux-kernel
The ability to detect if the system is running in a shared processor
mode is helpful in few more generic cases not just in
paravirtualization.
For example: At boot time, different scheduler/ topology flags may be
set based on the processor mode. Hence move it to a more generic file.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/paravirt.h | 12 ------------
arch/powerpc/include/asm/smp.h | 14 ++++++++++++++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index 0372b0093f72..cf83e837a571 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -15,13 +15,6 @@
#include <asm/kvm_guest.h>
#include <asm/cputhreads.h>
-DECLARE_STATIC_KEY_FALSE(shared_processor);
-
-static inline bool is_shared_processor(void)
-{
- return static_branch_unlikely(&shared_processor);
-}
-
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
extern struct static_key paravirt_steal_enabled;
extern struct static_key paravirt_steal_rq_enabled;
@@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
return lppaca_of(vcpu).idle;
}
#else
-static inline bool is_shared_processor(void)
-{
- return false;
-}
-
static inline u32 yield_count_of(int cpu)
{
return 0;
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index aaaa576d0e15..08631b2a4528 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -34,6 +34,20 @@ extern bool coregroup_enabled;
extern int cpu_to_chip_id(int cpu);
extern int *chip_id_lookup_table;
+#ifdef CONFIG_PPC_SPLPAR
+DECLARE_STATIC_KEY_FALSE(shared_processor);
+
+static inline bool is_shared_processor(void)
+{
+ return static_branch_unlikely(&shared_processor);
+}
+#else
+static inline bool is_shared_processor(void)
+{
+ return false;
+}
+#endif
+
DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Valentin Schneider, Srikar Dronamraju, Peter Zijlstra,
linux-kernel, x86, Ajay Kaher, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Nicholas Piggin,
Alexey Makhalov, linuxppc-dev, Josh Poimboeuf
The ability to detect if the system is running in a shared processor
mode is helpful in few more generic cases not just in
paravirtualization.
For example: At boot time, different scheduler/ topology flags may be
set based on the processor mode. Hence move it to a more generic file.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/paravirt.h | 12 ------------
arch/powerpc/include/asm/smp.h | 14 ++++++++++++++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index 0372b0093f72..cf83e837a571 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -15,13 +15,6 @@
#include <asm/kvm_guest.h>
#include <asm/cputhreads.h>
-DECLARE_STATIC_KEY_FALSE(shared_processor);
-
-static inline bool is_shared_processor(void)
-{
- return static_branch_unlikely(&shared_processor);
-}
-
#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
extern struct static_key paravirt_steal_enabled;
extern struct static_key paravirt_steal_rq_enabled;
@@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
return lppaca_of(vcpu).idle;
}
#else
-static inline bool is_shared_processor(void)
-{
- return false;
-}
-
static inline u32 yield_count_of(int cpu)
{
return 0;
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index aaaa576d0e15..08631b2a4528 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -34,6 +34,20 @@ extern bool coregroup_enabled;
extern int cpu_to_chip_id(int cpu);
extern int *chip_id_lookup_table;
+#ifdef CONFIG_PPC_SPLPAR
+DECLARE_STATIC_KEY_FALSE(shared_processor);
+
+static inline bool is_shared_processor(void)
+{
+ return static_branch_unlikely(&shared_processor);
+}
+#else
+static inline bool is_shared_processor(void)
+{
+ return false;
+}
+#endif
+
DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, Mark Rutland, linux-kernel
Like L2-cache info, coregroup information which is used to determine MC
sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
export coregroup information for shared processor LPARs. Hence disable
creating MC domains on shared LPAR Systems.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 498c2d51fc20..29da9262cb17 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
static bool has_coregroup_support(void)
{
+ /* Coregroup identification not available on shared systems */
+ if (is_shared_processor())
+ return 0;
+
return coregroup_enabled;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, linux-kernel, Rohan McLure, Nicholas Piggin,
linuxppc-dev, Josh Poimboeuf
Like L2-cache info, coregroup information which is used to determine MC
sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
export coregroup information for shared processor LPARs. Hence disable
creating MC domains on shared LPAR Systems.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 498c2d51fc20..29da9262cb17 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
static bool has_coregroup_support(void)
{
+ /* Coregroup identification not available on shared systems */
+ if (is_shared_processor())
+ return 0;
+
return coregroup_enabled;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, ndesaulniers, Mark Rutland,
linux-kernel
There are some variables that are only updated at boot time.
So add read_mostly attribute to such variables
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 29da9262cb17..b1eb11a66902 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -77,10 +77,10 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };
#endif
struct task_struct *secondary_current;
-bool has_big_cores;
-bool coregroup_enabled;
-bool thread_group_shares_l2;
-bool thread_group_shares_l3;
+bool has_big_cores __read_mostly;
+bool coregroup_enabled __read_mostly;
+bool thread_group_shares_l2 __read_mostly;
+bool thread_group_shares_l3 __read_mostly;
DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
@@ -987,7 +987,7 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
return 0;
}
-static bool shared_caches;
+static bool shared_caches __read_mostly;
DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
#ifdef CONFIG_SCHED_SMT
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, ndesaulniers, linux-kernel, Rohan McLure,
Nicholas Piggin, linuxppc-dev, Josh Poimboeuf
There are some variables that are only updated at boot time.
So add read_mostly attribute to such variables
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 29da9262cb17..b1eb11a66902 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -77,10 +77,10 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };
#endif
struct task_struct *secondary_current;
-bool has_big_cores;
-bool coregroup_enabled;
-bool thread_group_shares_l2;
-bool thread_group_shares_l3;
+bool has_big_cores __read_mostly;
+bool coregroup_enabled __read_mostly;
+bool thread_group_shares_l2 __read_mostly;
+bool thread_group_shares_l3 __read_mostly;
DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
@@ -987,7 +987,7 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
return 0;
}
-static bool shared_caches;
+static bool shared_caches __read_mostly;
DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
#ifdef CONFIG_SCHED_SMT
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 6/6] powerpc/smp: Avoid asym packing within thread_group of a core
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
@ 2023-10-18 16:37 ` Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
` (4 subsequent siblings)
5 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, ndesaulniers, Josh Poimboeuf, Mark Rutland,
linux-kernel
PowerVM Hypervisor will schedule at a core granularity. However each
core can have more than one thread_groups. For better utilization in
case of a shared processor, its preferable for the scheduler to pack to
the lowest core. However there is no benefit of moving a thread between
two thread groups of the same core.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b1eb11a66902..a710fb32a2a9 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1695,6 +1695,8 @@ void start_secondary(void *unused)
BUG();
}
+DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
+
static void __init fixup_topology(void)
{
int i;
@@ -1704,6 +1706,7 @@ static void __init fixup_topology(void)
static_branch_enable(&powerpc_asym_packing);
} else if (is_shared_processor() && has_big_cores) {
static_branch_enable(&powerpc_asym_packing);
+ static_branch_enable(&splpar_asym_pack);
}
#ifdef CONFIG_SCHED_SMT
@@ -1758,6 +1761,19 @@ void __init smp_cpus_done(unsigned int max_cpus)
set_sched_topology(powerpc_topology);
}
+/*
+ * For asym packing, by default lower numbered CPU has higher priority.
+ * On shared processors, pack to lower numbered core. However avoid moving
+ * between thread_groups within the same core.
+ */
+int arch_asym_cpu_priority(int cpu)
+{
+ if (static_branch_unlikely(&splpar_asym_pack))
+ return -cpu / threads_per_core;
+
+ return -cpu;
+}
+
#ifdef CONFIG_HOTPLUG_CPU
int __cpu_disable(void)
{
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH v2 6/6] powerpc/smp: Avoid asym packing within thread_group of a core
@ 2023-10-18 16:37 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-18 16:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, ndesaulniers, linux-kernel, Rohan McLure,
Nicholas Piggin, linuxppc-dev, Josh Poimboeuf
PowerVM Hypervisor will schedule at a core granularity. However each
core can have more than one thread_groups. For better utilization in
case of a shared processor, its preferable for the scheduler to pack to
the lowest core. However there is no benefit of moving a thread between
two thread groups of the same core.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b1eb11a66902..a710fb32a2a9 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1695,6 +1695,8 @@ void start_secondary(void *unused)
BUG();
}
+DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
+
static void __init fixup_topology(void)
{
int i;
@@ -1704,6 +1706,7 @@ static void __init fixup_topology(void)
static_branch_enable(&powerpc_asym_packing);
} else if (is_shared_processor() && has_big_cores) {
static_branch_enable(&powerpc_asym_packing);
+ static_branch_enable(&splpar_asym_pack);
}
#ifdef CONFIG_SCHED_SMT
@@ -1758,6 +1761,19 @@ void __init smp_cpus_done(unsigned int max_cpus)
set_sched_topology(powerpc_topology);
}
+/*
+ * For asym packing, by default lower numbered CPU has higher priority.
+ * On shared processors, pack to lower numbered core. However avoid moving
+ * between thread_groups within the same core.
+ */
+int arch_asym_cpu_priority(int cpu)
+{
+ if (static_branch_unlikely(&splpar_asym_pack))
+ return -cpu / threads_per_core;
+
+ return -cpu;
+}
+
#ifdef CONFIG_HOTPLUG_CPU
int __cpu_disable(void)
{
--
2.31.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 4:33 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:33 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, ndesaulniers, Mark Rutland,
linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> Currently cpu feature flag is checked whenever powerpc_smt_flags gets
> called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
> on the processor and all processors will either have this set or will
> have it unset.
The cpu_has_feature() test is implemented with a static key.
So AFAICS this is just replacing one static key with another?
I see that you use the new static key in subsequent patches. But
couldn't those just use the existing cpu feature test?
Anyway I'd be interested to see how the generated code differs
before/after this.
cheers
> Hence only check for the feature flag once and cache it to be used
> subsequently. This commit will help avoid a branch in powerpc_smt_flags
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2: Using static keys instead of a variable.
> Using pr_info_once instead of printk
>
> arch/powerpc/kernel/smp.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5826f5108a12..37c41297c9ce 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -988,18 +988,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
> }
>
> static bool shared_caches;
> +DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
>
> #ifdef CONFIG_SCHED_SMT
> /* cpumask of CPUs with asymmetric SMT dependency */
> static int powerpc_smt_flags(void)
> {
> - int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
>
> - if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> - printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> - flags |= SD_ASYM_PACKING;
> - }
> - return flags;
> + return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> }
> #endif
>
> @@ -1686,6 +1684,11 @@ static void __init fixup_topology(void)
> {
> int i;
>
> + if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> + pr_info_once("Enabling Asymmetric SMT scheduling\n");
> + static_branch_enable(&powerpc_asym_packing);
> + }
> +
> #ifdef CONFIG_SCHED_SMT
> if (has_big_cores) {
> pr_info("Big cores detected but using small core scheduling\n");
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
@ 2023-10-19 4:33 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:33 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, ndesaulniers, linux-kernel, Rohan McLure,
Nicholas Piggin, linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> Currently cpu feature flag is checked whenever powerpc_smt_flags gets
> called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
> on the processor and all processors will either have this set or will
> have it unset.
The cpu_has_feature() test is implemented with a static key.
So AFAICS this is just replacing one static key with another?
I see that you use the new static key in subsequent patches. But
couldn't those just use the existing cpu feature test?
Anyway I'd be interested to see how the generated code differs
before/after this.
cheers
> Hence only check for the feature flag once and cache it to be used
> subsequently. This commit will help avoid a branch in powerpc_smt_flags
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2: Using static keys instead of a variable.
> Using pr_info_once instead of printk
>
> arch/powerpc/kernel/smp.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5826f5108a12..37c41297c9ce 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -988,18 +988,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
> }
>
> static bool shared_caches;
> +DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
>
> #ifdef CONFIG_SCHED_SMT
> /* cpumask of CPUs with asymmetric SMT dependency */
> static int powerpc_smt_flags(void)
> {
> - int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
>
> - if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> - printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> - flags |= SD_ASYM_PACKING;
> - }
> - return flags;
> + return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> }
> #endif
>
> @@ -1686,6 +1684,11 @@ static void __init fixup_topology(void)
> {
> int i;
>
> + if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> + pr_info_once("Enabling Asymmetric SMT scheduling\n");
> + static_branch_enable(&powerpc_asym_packing);
> + }
> +
> #ifdef CONFIG_SCHED_SMT
> if (has_big_cores) {
> pr_info("Big cores detected but using small core scheduling\n");
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 4:38 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:38 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, Mark Rutland, linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> If there are shared processor LPARs, underlying Hypervisor can have more
> virtual cores to handle than actual physical cores.
>
> Starting with Power 9, a core has 2 nearly independent thread groups.
You need to be clearer here that you're talking about "big cores", not
SMT4 cores as seen on bare metal systems.
> On a shared processors LPARs, it helps to pack threads to lesser number
> of cores so that the overall system performance and utilization
> improves. PowerVM schedules at a core level. Hence packing to fewer
> cores helps.
>
> For example: Lets says there are two 8-core Shared LPARs that are
> actually sharing a 8 Core shared physical pool, each running 8 threads
> each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> them to perform better. This is because each of the LPAR will get
> 100% time to run applications and there will no switching required by
> the Hypervisor.
>
> To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
.. when the system is running in shared processor mode and has big cores.
cheers
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 37c41297c9ce..498c2d51fc20 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> */
> static int powerpc_shared_cache_flags(void)
> {
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> +
> return SD_SHARE_PKG_RESOURCES;
> }
>
> +static int powerpc_shared_proc_flags(void)
> +{
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_ASYM_PACKING;
> +
> + return 0;
> +}
> +
> /*
> * We can't just pass cpu_l2_cache_mask() directly because
> * returns a non-const pointer and the compiler barfs on that.
> @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> #endif
> { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> - { cpu_mc_mask, SD_INIT_NAME(MC) },
> - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> { NULL, },
> };
>
> @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> pr_info_once("Enabling Asymmetric SMT scheduling\n");
> static_branch_enable(&powerpc_asym_packing);
> + } else if (is_shared_processor() && has_big_cores) {
> + static_branch_enable(&powerpc_asym_packing);
> }
>
> #ifdef CONFIG_SCHED_SMT
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-19 4:38 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:38 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, linux-kernel, Rohan McLure, Nicholas Piggin,
linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> If there are shared processor LPARs, underlying Hypervisor can have more
> virtual cores to handle than actual physical cores.
>
> Starting with Power 9, a core has 2 nearly independent thread groups.
You need to be clearer here that you're talking about "big cores", not
SMT4 cores as seen on bare metal systems.
> On a shared processors LPARs, it helps to pack threads to lesser number
> of cores so that the overall system performance and utilization
> improves. PowerVM schedules at a core level. Hence packing to fewer
> cores helps.
>
> For example: Lets says there are two 8-core Shared LPARs that are
> actually sharing a 8 Core shared physical pool, each running 8 threads
> each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> them to perform better. This is because each of the LPAR will get
> 100% time to run applications and there will no switching required by
> the Hypervisor.
>
> To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
.. when the system is running in shared processor mode and has big cores.
cheers
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 37c41297c9ce..498c2d51fc20 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> */
> static int powerpc_shared_cache_flags(void)
> {
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> +
> return SD_SHARE_PKG_RESOURCES;
> }
>
> +static int powerpc_shared_proc_flags(void)
> +{
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_ASYM_PACKING;
> +
> + return 0;
> +}
> +
> /*
> * We can't just pass cpu_l2_cache_mask() directly because
> * returns a non-const pointer and the compiler barfs on that.
> @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> #endif
> { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> - { cpu_mc_mask, SD_INIT_NAME(MC) },
> - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> { NULL, },
> };
>
> @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> pr_info_once("Enabling Asymmetric SMT scheduling\n");
> static_branch_enable(&powerpc_asym_packing);
> + } else if (is_shared_processor() && has_big_cores) {
> + static_branch_enable(&powerpc_asym_packing);
> }
>
> #ifdef CONFIG_SCHED_SMT
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
2023-10-18 16:37 ` Srikar Dronamraju
(?)
@ 2023-10-19 4:41 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:41 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Ajay Kaher, Alexey Makhalov,
VMware PV-Drivers Reviewers, Josh Poimboeuf, virtualization, x86,
linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> The ability to detect if the system is running in a shared processor
> mode is helpful in few more generic cases not just in
> paravirtualization.
> For example: At boot time, different scheduler/ topology flags may be
> set based on the processor mode. Hence move it to a more generic file.
I'd rather you just included paravirt.h in the few files where you need it.
cheers
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 0372b0093f72..cf83e837a571 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -15,13 +15,6 @@
> #include <asm/kvm_guest.h>
> #include <asm/cputhreads.h>
>
> -DECLARE_STATIC_KEY_FALSE(shared_processor);
> -
> -static inline bool is_shared_processor(void)
> -{
> - return static_branch_unlikely(&shared_processor);
> -}
> -
> #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> extern struct static_key paravirt_steal_enabled;
> extern struct static_key paravirt_steal_rq_enabled;
> @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> return lppaca_of(vcpu).idle;
> }
> #else
> -static inline bool is_shared_processor(void)
> -{
> - return false;
> -}
> -
> static inline u32 yield_count_of(int cpu)
> {
> return 0;
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index aaaa576d0e15..08631b2a4528 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> extern int cpu_to_chip_id(int cpu);
> extern int *chip_id_lookup_table;
>
> +#ifdef CONFIG_PPC_SPLPAR
> +DECLARE_STATIC_KEY_FALSE(shared_processor);
> +
> +static inline bool is_shared_processor(void)
> +{
> + return static_branch_unlikely(&shared_processor);
> +}
> +#else
> +static inline bool is_shared_processor(void)
> +{
> + return false;
> +}
> +#endif
> +
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-19 4:41 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:41 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Srikar Dronamraju, Peter Zijlstra,
linux-kernel, x86, Ajay Kaher, Christophe Leroy, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Nicholas Piggin,
Alexey Makhalov, linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> The ability to detect if the system is running in a shared processor
> mode is helpful in few more generic cases not just in
> paravirtualization.
> For example: At boot time, different scheduler/ topology flags may be
> set based on the processor mode. Hence move it to a more generic file.
I'd rather you just included paravirt.h in the few files where you need it.
cheers
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 0372b0093f72..cf83e837a571 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -15,13 +15,6 @@
> #include <asm/kvm_guest.h>
> #include <asm/cputhreads.h>
>
> -DECLARE_STATIC_KEY_FALSE(shared_processor);
> -
> -static inline bool is_shared_processor(void)
> -{
> - return static_branch_unlikely(&shared_processor);
> -}
> -
> #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> extern struct static_key paravirt_steal_enabled;
> extern struct static_key paravirt_steal_rq_enabled;
> @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> return lppaca_of(vcpu).idle;
> }
> #else
> -static inline bool is_shared_processor(void)
> -{
> - return false;
> -}
> -
> static inline u32 yield_count_of(int cpu)
> {
> return 0;
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index aaaa576d0e15..08631b2a4528 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> extern int cpu_to_chip_id(int cpu);
> extern int *chip_id_lookup_table;
>
> +#ifdef CONFIG_PPC_SPLPAR
> +DECLARE_STATIC_KEY_FALSE(shared_processor);
> +
> +static inline bool is_shared_processor(void)
> +{
> + return static_branch_unlikely(&shared_processor);
> +}
> +#else
> +static inline bool is_shared_processor(void)
> +{
> + return false;
> +}
> +#endif
> +
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
> --
> 2.31.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-19 4:41 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:41 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Srikar Dronamraju, Peter Zijlstra,
linux-kernel, x86, Ajay Kaher, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Nicholas Piggin,
Alexey Makhalov, linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> The ability to detect if the system is running in a shared processor
> mode is helpful in few more generic cases not just in
> paravirtualization.
> For example: At boot time, different scheduler/ topology flags may be
> set based on the processor mode. Hence move it to a more generic file.
I'd rather you just included paravirt.h in the few files where you need it.
cheers
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 0372b0093f72..cf83e837a571 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -15,13 +15,6 @@
> #include <asm/kvm_guest.h>
> #include <asm/cputhreads.h>
>
> -DECLARE_STATIC_KEY_FALSE(shared_processor);
> -
> -static inline bool is_shared_processor(void)
> -{
> - return static_branch_unlikely(&shared_processor);
> -}
> -
> #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> extern struct static_key paravirt_steal_enabled;
> extern struct static_key paravirt_steal_rq_enabled;
> @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> return lppaca_of(vcpu).idle;
> }
> #else
> -static inline bool is_shared_processor(void)
> -{
> - return false;
> -}
> -
> static inline u32 yield_count_of(int cpu)
> {
> return 0;
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index aaaa576d0e15..08631b2a4528 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> extern int cpu_to_chip_id(int cpu);
> extern int *chip_id_lookup_table;
>
> +#ifdef CONFIG_PPC_SPLPAR
> +DECLARE_STATIC_KEY_FALSE(shared_processor);
> +
> +static inline bool is_shared_processor(void)
> +{
> + return static_branch_unlikely(&shared_processor);
> +}
> +#else
> +static inline bool is_shared_processor(void)
> +{
> + return false;
> +}
> +#endif
> +
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 4:48 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:48 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, Mark Rutland, linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> Like L2-cache info, coregroup information which is used to determine MC
> sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> export coregroup information for shared processor LPARs. Hence disable
> creating MC domains on shared LPAR Systems.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/smp.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 498c2d51fc20..29da9262cb17 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
>
> static bool has_coregroup_support(void)
> {
> + /* Coregroup identification not available on shared systems */
> + if (is_shared_processor())
> + return 0;
That will catch guests running under KVM too right? Do we want that?
> return coregroup_enabled;
What does coregroup_enabled mean now?
I'd rather this was actually checking the presence of something, rather
than just hard coding that shared processor means no coregroup support.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
@ 2023-10-19 4:48 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:48 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, linux-kernel, Rohan McLure, Nicholas Piggin,
linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> Like L2-cache info, coregroup information which is used to determine MC
> sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> export coregroup information for shared processor LPARs. Hence disable
> creating MC domains on shared LPAR Systems.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/smp.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 498c2d51fc20..29da9262cb17 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
>
> static bool has_coregroup_support(void)
> {
> + /* Coregroup identification not available on shared systems */
> + if (is_shared_processor())
> + return 0;
That will catch guests running under KVM too right? Do we want that?
> return coregroup_enabled;
What does coregroup_enabled mean now?
I'd rather this was actually checking the presence of something, rather
than just hard coding that shared processor means no coregroup support.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 4:49 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:49 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Srikar Dronamraju, Christophe Leroy,
Nicholas Piggin, Peter Zijlstra, Rohan McLure,
Valentin Schneider, Josh Poimboeuf, ndesaulniers, Mark Rutland,
linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> There are some variables that are only updated at boot time.
> So add read_mostly attribute to such variables
If they're only updated at boot time then __ro_after_init would be the
better annotation.
cheers
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 29da9262cb17..b1eb11a66902 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -77,10 +77,10 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };
> #endif
>
> struct task_struct *secondary_current;
> -bool has_big_cores;
> -bool coregroup_enabled;
> -bool thread_group_shares_l2;
> -bool thread_group_shares_l3;
> +bool has_big_cores __read_mostly;
> +bool coregroup_enabled __read_mostly;
> +bool thread_group_shares_l2 __read_mostly;
> +bool thread_group_shares_l3 __read_mostly;
>
> DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> @@ -987,7 +987,7 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
> return 0;
> }
>
> -static bool shared_caches;
> +static bool shared_caches __read_mostly;
> DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
>
> #ifdef CONFIG_SCHED_SMT
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
@ 2023-10-19 4:49 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 4:49 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
Peter Zijlstra, ndesaulniers, linux-kernel, Rohan McLure,
Nicholas Piggin, linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> There are some variables that are only updated at boot time.
> So add read_mostly attribute to such variables
If they're only updated at boot time then __ro_after_init would be the
better annotation.
cheers
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 29da9262cb17..b1eb11a66902 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -77,10 +77,10 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };
> #endif
>
> struct task_struct *secondary_current;
> -bool has_big_cores;
> -bool coregroup_enabled;
> -bool thread_group_shares_l2;
> -bool thread_group_shares_l3;
> +bool has_big_cores __read_mostly;
> +bool coregroup_enabled __read_mostly;
> +bool thread_group_shares_l2 __read_mostly;
> +bool thread_group_shares_l3 __read_mostly;
>
> DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> @@ -987,7 +987,7 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
> return 0;
> }
>
> -static bool shared_caches;
> +static bool shared_caches __read_mostly;
> DEFINE_STATIC_KEY_FALSE(powerpc_asym_packing);
>
> #ifdef CONFIG_SCHED_SMT
> --
> 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-19 4:38 ` Michael Ellerman
@ 2023-10-19 7:48 ` Peter Zijlstra
-1 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:48 UTC (permalink / raw)
To: Michael Ellerman
Cc: Srikar Dronamraju, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, Mark Rutland, linux-kernel
On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > If there are shared processor LPARs, underlying Hypervisor can have more
> > virtual cores to handle than actual physical cores.
> >
> > Starting with Power 9, a core has 2 nearly independent thread groups.
>
> You need to be clearer here that you're talking about "big cores", not
> SMT4 cores as seen on bare metal systems.
What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
went that route (yet?).. help?
> > On a shared processors LPARs, it helps to pack threads to lesser number
> > of cores so that the overall system performance and utilization
> > improves. PowerVM schedules at a core level. Hence packing to fewer
> > cores helps.
> >
> > For example: Lets says there are two 8-core Shared LPARs that are
> > actually sharing a 8 Core shared physical pool, each running 8 threads
> > each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> > them to perform better. This is because each of the LPAR will get
> > 100% time to run applications and there will no switching required by
> > the Hypervisor.
> >
> > To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
>
> .. when the system is running in shared processor mode and has big cores.
>
> cheers
>
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 37c41297c9ce..498c2d51fc20 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> > */
> > static int powerpc_shared_cache_flags(void)
> > {
> > + if (static_branch_unlikely(&powerpc_asym_packing))
> > + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> > +
> > return SD_SHARE_PKG_RESOURCES;
> > }
> >
> > +static int powerpc_shared_proc_flags(void)
> > +{
> > + if (static_branch_unlikely(&powerpc_asym_packing))
> > + return SD_ASYM_PACKING;
> > +
> > + return 0;
> > +}
Can you leave the future reader a clue in the form of a comment around
here perhaps? Explaining *why* things are as they are etc..
> > +
> > /*
> > * We can't just pass cpu_l2_cache_mask() directly because
> > * returns a non-const pointer and the compiler barfs on that.
> > @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> > { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> > #endif
> > { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> > - { cpu_mc_mask, SD_INIT_NAME(MC) },
> > - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> > + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> > + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> > { NULL, },
> > };
> >
> > @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> > if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> > pr_info_once("Enabling Asymmetric SMT scheduling\n");
> > static_branch_enable(&powerpc_asym_packing);
> > + } else if (is_shared_processor() && has_big_cores) {
> > + static_branch_enable(&powerpc_asym_packing);
> > }
> >
> > #ifdef CONFIG_SCHED_SMT
> > --
> > 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-19 7:48 ` Peter Zijlstra
0 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:48 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
linux-kernel, Rohan McLure, Nicholas Piggin, linuxppc-dev,
Josh Poimboeuf
On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > If there are shared processor LPARs, underlying Hypervisor can have more
> > virtual cores to handle than actual physical cores.
> >
> > Starting with Power 9, a core has 2 nearly independent thread groups.
>
> You need to be clearer here that you're talking about "big cores", not
> SMT4 cores as seen on bare metal systems.
What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
went that route (yet?).. help?
> > On a shared processors LPARs, it helps to pack threads to lesser number
> > of cores so that the overall system performance and utilization
> > improves. PowerVM schedules at a core level. Hence packing to fewer
> > cores helps.
> >
> > For example: Lets says there are two 8-core Shared LPARs that are
> > actually sharing a 8 Core shared physical pool, each running 8 threads
> > each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> > them to perform better. This is because each of the LPAR will get
> > 100% time to run applications and there will no switching required by
> > the Hypervisor.
> >
> > To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
>
> .. when the system is running in shared processor mode and has big cores.
>
> cheers
>
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 37c41297c9ce..498c2d51fc20 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> > */
> > static int powerpc_shared_cache_flags(void)
> > {
> > + if (static_branch_unlikely(&powerpc_asym_packing))
> > + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> > +
> > return SD_SHARE_PKG_RESOURCES;
> > }
> >
> > +static int powerpc_shared_proc_flags(void)
> > +{
> > + if (static_branch_unlikely(&powerpc_asym_packing))
> > + return SD_ASYM_PACKING;
> > +
> > + return 0;
> > +}
Can you leave the future reader a clue in the form of a comment around
here perhaps? Explaining *why* things are as they are etc..
> > +
> > /*
> > * We can't just pass cpu_l2_cache_mask() directly because
> > * returns a non-const pointer and the compiler barfs on that.
> > @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> > { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> > #endif
> > { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> > - { cpu_mc_mask, SD_INIT_NAME(MC) },
> > - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> > + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> > + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> > { NULL, },
> > };
> >
> > @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> > if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> > pr_info_once("Enabling Asymmetric SMT scheduling\n");
> > static_branch_enable(&powerpc_asym_packing);
> > + } else if (is_shared_processor() && has_big_cores) {
> > + static_branch_enable(&powerpc_asym_packing);
> > }
> >
> > #ifdef CONFIG_SCHED_SMT
> > --
> > 2.31.1
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
2023-10-19 4:48 ` Michael Ellerman
@ 2023-10-19 7:50 ` Peter Zijlstra
-1 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Srikar Dronamraju, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, Mark Rutland, linux-kernel
On Thu, Oct 19, 2023 at 03:48:48PM +1100, Michael Ellerman wrote:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Like L2-cache info, coregroup information which is used to determine MC
> > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > export coregroup information for shared processor LPARs. Hence disable
> > creating MC domains on shared LPAR Systems.
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/smp.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 498c2d51fc20..29da9262cb17 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> >
> > static bool has_coregroup_support(void)
> > {
> > + /* Coregroup identification not available on shared systems */
> > + if (is_shared_processor())
> > + return 0;
>
> That will catch guests running under KVM too right? Do we want that?
Some KVM people use vcpu pinning and pass-through topology things,
slice-of-hardware or something like that. In that scenario you actively
do want this.
I'm fairly clueless on when this is_shared_processor() gets to be true,
so that might already be dealt with..
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
@ 2023-10-19 7:50 ` Peter Zijlstra
0 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
linux-kernel, Rohan McLure, Nicholas Piggin, linuxppc-dev,
Josh Poimboeuf
On Thu, Oct 19, 2023 at 03:48:48PM +1100, Michael Ellerman wrote:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Like L2-cache info, coregroup information which is used to determine MC
> > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > export coregroup information for shared processor LPARs. Hence disable
> > creating MC domains on shared LPAR Systems.
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/smp.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 498c2d51fc20..29da9262cb17 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> >
> > static bool has_coregroup_support(void)
> > {
> > + /* Coregroup identification not available on shared systems */
> > + if (is_shared_processor())
> > + return 0;
>
> That will catch guests running under KVM too right? Do we want that?
Some KVM people use vcpu pinning and pass-through topology things,
slice-of-hardware or something like that. In that scenario you actively
do want this.
I'm fairly clueless on when this is_shared_processor() gets to be true,
so that might already be dealt with..
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 7:51 ` Peter Zijlstra
-1 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:51 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Michael Ellerman, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, ndesaulniers, Mark Rutland, linux-kernel
On Wed, Oct 18, 2023 at 10:07:45PM +0530, Srikar Dronamraju wrote:
> There are some variables that are only updated at boot time.
> So add read_mostly attribute to such variables
You don't have __ro_after_init ?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
@ 2023-10-19 7:51 ` Peter Zijlstra
0 siblings, 0 replies; 53+ messages in thread
From: Peter Zijlstra @ 2023-10-19 7:51 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, ndesaulniers, linux-kernel,
Nicholas Piggin, Rohan McLure, linuxppc-dev, Josh Poimboeuf
On Wed, Oct 18, 2023 at 10:07:45PM +0530, Srikar Dronamraju wrote:
> There are some variables that are only updated at boot time.
> So add read_mostly attribute to such variables
You don't have __ro_after_init ?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 10:30 ` Shrikanth Hegde
-1 siblings, 0 replies; 53+ messages in thread
From: Shrikanth Hegde @ 2023-10-19 10:30 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Peter Zijlstra, linux-kernel, x86,
Ajay Kaher, virtualization, VMware PV-Drivers Reviewers,
Rohan McLure, Nicholas Piggin, Alexey Makhalov, linuxppc-dev,
Josh Poimboeuf, Michael Ellerman
On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> The ability to detect if the system is running in a shared processor
> mode is helpful in few more generic cases not just in
> paravirtualization.
> For example: At boot time, different scheduler/ topology flags may be
> set based on the processor mode. Hence move it to a more generic file.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/paravirt.h | 12 ------------
> arch/powerpc/include/asm/smp.h | 14 ++++++++++++++
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 0372b0093f72..cf83e837a571 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -15,13 +15,6 @@
> #include <asm/kvm_guest.h>
> #include <asm/cputhreads.h>
>
> -DECLARE_STATIC_KEY_FALSE(shared_processor);
> -
> -static inline bool is_shared_processor(void)
> -{
> - return static_branch_unlikely(&shared_processor);
> -}
> -
> #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> extern struct static_key paravirt_steal_enabled;
> extern struct static_key paravirt_steal_rq_enabled;
> @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> return lppaca_of(vcpu).idle;
> }
Hi Srikar,
This patch fails to apply on tip/master as it depends on
https://lore.kernel.org/all/20231019091452.95260-1-srikar@linux.vnet.ibm.com/ to be applied first.
> #else
> -static inline bool is_shared_processor(void)
> -{
> - return false;
> -}
> -
> static inline u32 yield_count_of(int cpu)
> {
> return 0;
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index aaaa576d0e15..08631b2a4528 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> extern int cpu_to_chip_id(int cpu);
> extern int *chip_id_lookup_table;
>
> +#ifdef CONFIG_PPC_SPLPAR
> +DECLARE_STATIC_KEY_FALSE(shared_processor);
> +
> +static inline bool is_shared_processor(void)
> +{
> + return static_branch_unlikely(&shared_processor);
> +}
> +#else
> +static inline bool is_shared_processor(void)
> +{
> + return false;
> +}
> +#endif
> +
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-19 10:30 ` Shrikanth Hegde
0 siblings, 0 replies; 53+ messages in thread
From: Shrikanth Hegde @ 2023-10-19 10:30 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Peter Zijlstra, x86, linux-kernel,
Nicholas Piggin, virtualization, VMware PV-Drivers Reviewers,
Ajay Kaher, Rohan McLure, Alexey Makhalov, linuxppc-dev,
Josh Poimboeuf
On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> The ability to detect if the system is running in a shared processor
> mode is helpful in few more generic cases not just in
> paravirtualization.
> For example: At boot time, different scheduler/ topology flags may be
> set based on the processor mode. Hence move it to a more generic file.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/paravirt.h | 12 ------------
> arch/powerpc/include/asm/smp.h | 14 ++++++++++++++
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 0372b0093f72..cf83e837a571 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -15,13 +15,6 @@
> #include <asm/kvm_guest.h>
> #include <asm/cputhreads.h>
>
> -DECLARE_STATIC_KEY_FALSE(shared_processor);
> -
> -static inline bool is_shared_processor(void)
> -{
> - return static_branch_unlikely(&shared_processor);
> -}
> -
> #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> extern struct static_key paravirt_steal_enabled;
> extern struct static_key paravirt_steal_rq_enabled;
> @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> return lppaca_of(vcpu).idle;
> }
Hi Srikar,
This patch fails to apply on tip/master as it depends on
https://lore.kernel.org/all/20231019091452.95260-1-srikar@linux.vnet.ibm.com/ to be applied first.
> #else
> -static inline bool is_shared_processor(void)
> -{
> - return false;
> -}
> -
> static inline u32 yield_count_of(int cpu)
> {
> return 0;
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index aaaa576d0e15..08631b2a4528 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> extern int cpu_to_chip_id(int cpu);
> extern int *chip_id_lookup_table;
>
> +#ifdef CONFIG_PPC_SPLPAR
> +DECLARE_STATIC_KEY_FALSE(shared_processor);
> +
> +static inline bool is_shared_processor(void)
> +{
> + return static_branch_unlikely(&shared_processor);
> +}
> +#else
> +static inline bool is_shared_processor(void)
> +{
> + return false;
> +}
> +#endif
> +
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-19 7:48 ` Peter Zijlstra
@ 2023-10-19 11:50 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 11:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Srikar Dronamraju, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, Mark Rutland, linux-kernel
Peter Zijlstra <peterz@infradead.org> writes:
> On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
>> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > If there are shared processor LPARs, underlying Hypervisor can have more
>> > virtual cores to handle than actual physical cores.
>> >
>> > Starting with Power 9, a core has 2 nearly independent thread groups.
>>
>> You need to be clearer here that you're talking about "big cores", not
>> SMT4 cores as seen on bare metal systems.
>
> What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
> went that route (yet?).. help?
No it's not big.LITTLE :)
It means we have two SMT4 cores glued together that behave as a single
SMT8 core, a system is either in "big core" mode or it's not, it's never
heterogeneous.
If you grep for "big_core" there's some code in the kernel for dealing
with it, though it's probably not very illuminating.
Possibly we should switch to using the "thread group" terminology, so
that it doesn't confuse folks about big.LITTLE. Also the device tree
property that we use to discover if we're using big cores is called
ibm,thread-groups.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-19 11:50 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-19 11:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Valentin Schneider, Srikar Dronamraju,
linux-kernel, Rohan McLure, Nicholas Piggin, linuxppc-dev,
Josh Poimboeuf
Peter Zijlstra <peterz@infradead.org> writes:
> On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
>> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > If there are shared processor LPARs, underlying Hypervisor can have more
>> > virtual cores to handle than actual physical cores.
>> >
>> > Starting with Power 9, a core has 2 nearly independent thread groups.
>>
>> You need to be clearer here that you're talking about "big cores", not
>> SMT4 cores as seen on bare metal systems.
>
> What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
> went that route (yet?).. help?
No it's not big.LITTLE :)
It means we have two SMT4 cores glued together that behave as a single
SMT8 core, a system is either in "big core" mode or it's not, it's never
heterogeneous.
If you grep for "big_core" there's some code in the kernel for dealing
with it, though it's probably not very illuminating.
Possibly we should switch to using the "thread group" terminology, so
that it doesn't confuse folks about big.LITTLE. Also the device tree
property that we use to discover if we're using big cores is called
ibm,thread-groups.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-19 7:48 ` Peter Zijlstra
@ 2023-10-19 12:54 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 12:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Valentin Schneider, linux-kernel, Nicholas Piggin,
Rohan McLure, linuxppc-dev, Josh Poimboeuf
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:48:28]:
> On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
> > Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > > If there are shared processor LPARs, underlying Hypervisor can have more
> > > virtual cores to handle than actual physical cores.
> > >
> > > Starting with Power 9, a core has 2 nearly independent thread groups.
> >
> > You need to be clearer here that you're talking about "big cores", not
> > SMT4 cores as seen on bare metal systems.
>
> What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
> went that route (yet?).. help?
>
Each independent thread group acts as a SMT4 core or a small core. A set of
2 thread groups form a SMT8 core aka big core. PowerVM aka pHYp schedules
at a big core granularity
So if we have 2 LPARS, each spanning 2 big cores, aka 16 CPUs, and if at
somepoint, each LPAR has only 2 threads to run, we are exploring if we can
run both the threads on just one big core, so that PhyP can schedule both
LPARS at the same time and avoid having to switch/multiplex between these
two LPARS.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-19 12:54 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 12:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Ellerman, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, Mark Rutland, linux-kernel
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:48:28]:
> On Thu, Oct 19, 2023 at 03:38:40PM +1100, Michael Ellerman wrote:
> > Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > > If there are shared processor LPARs, underlying Hypervisor can have more
> > > virtual cores to handle than actual physical cores.
> > >
> > > Starting with Power 9, a core has 2 nearly independent thread groups.
> >
> > You need to be clearer here that you're talking about "big cores", not
> > SMT4 cores as seen on bare metal systems.
>
> What is a 'big core' ? I'm thinking big.LITTLE, but I didn't think Power
> went that route (yet?).. help?
>
Each independent thread group acts as a SMT4 core or a small core. A set of
2 thread groups form a SMT8 core aka big core. PowerVM aka pHYp schedules
at a big core granularity
So if we have 2 LPARS, each spanning 2 big cores, aka 16 CPUs, and if at
somepoint, each LPAR has only 2 threads to run, we are exploring if we can
run both the threads on just one big core, so that PhyP can schedule both
LPARS at the same time and avoid having to switch/multiplex between these
two LPARS.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
2023-10-19 7:51 ` Peter Zijlstra
@ 2023-10-19 12:56 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 12:56 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Valentin Schneider, ndesaulniers, linux-kernel,
Nicholas Piggin, Rohan McLure, linuxppc-dev, Josh Poimboeuf
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:51:27]:
> On Wed, Oct 18, 2023 at 10:07:45PM +0530, Srikar Dronamraju wrote:
> > There are some variables that are only updated at boot time.
> > So add read_mostly attribute to such variables
>
> You don't have __ro_after_init ?
Michael also responded with the same input, will change over to
__ro_after_init in the next iteration.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute
@ 2023-10-19 12:56 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 12:56 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Ellerman, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, ndesaulniers, Mark Rutland, linux-kernel
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:51:27]:
> On Wed, Oct 18, 2023 at 10:07:45PM +0530, Srikar Dronamraju wrote:
> > There are some variables that are only updated at boot time.
> > So add read_mostly attribute to such variables
>
> You don't have __ro_after_init ?
Michael also responded with the same input, will change over to
__ro_after_init in the next iteration.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
2023-10-19 4:41 ` Michael Ellerman
@ 2023-10-19 13:08 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:08 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Christophe Leroy, Nicholas Piggin, Peter Zijlstra,
Rohan McLure, Valentin Schneider, Ajay Kaher, Alexey Makhalov,
VMware PV-Drivers Reviewers, Josh Poimboeuf, virtualization, x86,
linux-kernel
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:41:40]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > The ability to detect if the system is running in a shared processor
> > mode is helpful in few more generic cases not just in
> > paravirtualization.
> > For example: At boot time, different scheduler/ topology flags may be
> > set based on the processor mode. Hence move it to a more generic file.
>
> I'd rather you just included paravirt.h in the few files where you need it.
I thought, detecting if a Processor was shared or not was more a
smp/processor related than a paravirt related.
Will drop as suggested.
>
> cheers
>
> > diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> > index 0372b0093f72..cf83e837a571 100644
> > --- a/arch/powerpc/include/asm/paravirt.h
> > +++ b/arch/powerpc/include/asm/paravirt.h
> > @@ -15,13 +15,6 @@
> > #include <asm/kvm_guest.h>
> > #include <asm/cputhreads.h>
> >
> > -DECLARE_STATIC_KEY_FALSE(shared_processor);
> > -
> > -static inline bool is_shared_processor(void)
> > -{
> > - return static_branch_unlikely(&shared_processor);
> > -}
> > -
> > #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> > extern struct static_key paravirt_steal_enabled;
> > extern struct static_key paravirt_steal_rq_enabled;
> > @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> > return lppaca_of(vcpu).idle;
> > }
> > #else
> > -static inline bool is_shared_processor(void)
> > -{
> > - return false;
> > -}
> > -
> > static inline u32 yield_count_of(int cpu)
> > {
> > return 0;
> > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> > index aaaa576d0e15..08631b2a4528 100644
> > --- a/arch/powerpc/include/asm/smp.h
> > +++ b/arch/powerpc/include/asm/smp.h
> > @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> > extern int cpu_to_chip_id(int cpu);
> > extern int *chip_id_lookup_table;
> >
> > +#ifdef CONFIG_PPC_SPLPAR
> > +DECLARE_STATIC_KEY_FALSE(shared_processor);
> > +
> > +static inline bool is_shared_processor(void)
> > +{
> > + return static_branch_unlikely(&shared_processor);
> > +}
> > +#else
> > +static inline bool is_shared_processor(void)
> > +{
> > + return false;
> > +}
> > +#endif
> > +
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
> > --
> > 2.31.1
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-19 13:08 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:08 UTC (permalink / raw)
To: Michael Ellerman
Cc: Valentin Schneider, Peter Zijlstra, linux-kernel, x86,
Ajay Kaher, Nicholas Piggin, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Alexey Makhalov,
linuxppc-dev, Josh Poimboeuf
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:41:40]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > The ability to detect if the system is running in a shared processor
> > mode is helpful in few more generic cases not just in
> > paravirtualization.
> > For example: At boot time, different scheduler/ topology flags may be
> > set based on the processor mode. Hence move it to a more generic file.
>
> I'd rather you just included paravirt.h in the few files where you need it.
I thought, detecting if a Processor was shared or not was more a
smp/processor related than a paravirt related.
Will drop as suggested.
>
> cheers
>
> > diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> > index 0372b0093f72..cf83e837a571 100644
> > --- a/arch/powerpc/include/asm/paravirt.h
> > +++ b/arch/powerpc/include/asm/paravirt.h
> > @@ -15,13 +15,6 @@
> > #include <asm/kvm_guest.h>
> > #include <asm/cputhreads.h>
> >
> > -DECLARE_STATIC_KEY_FALSE(shared_processor);
> > -
> > -static inline bool is_shared_processor(void)
> > -{
> > - return static_branch_unlikely(&shared_processor);
> > -}
> > -
> > #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
> > extern struct static_key paravirt_steal_enabled;
> > extern struct static_key paravirt_steal_rq_enabled;
> > @@ -77,11 +70,6 @@ static inline bool is_vcpu_idle(int vcpu)
> > return lppaca_of(vcpu).idle;
> > }
> > #else
> > -static inline bool is_shared_processor(void)
> > -{
> > - return false;
> > -}
> > -
> > static inline u32 yield_count_of(int cpu)
> > {
> > return 0;
> > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> > index aaaa576d0e15..08631b2a4528 100644
> > --- a/arch/powerpc/include/asm/smp.h
> > +++ b/arch/powerpc/include/asm/smp.h
> > @@ -34,6 +34,20 @@ extern bool coregroup_enabled;
> > extern int cpu_to_chip_id(int cpu);
> > extern int *chip_id_lookup_table;
> >
> > +#ifdef CONFIG_PPC_SPLPAR
> > +DECLARE_STATIC_KEY_FALSE(shared_processor);
> > +
> > +static inline bool is_shared_processor(void)
> > +{
> > + return static_branch_unlikely(&shared_processor);
> > +}
> > +#else
> > +static inline bool is_shared_processor(void)
> > +{
> > + return false;
> > +}
> > +#endif
> > +
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l1_cache_map);
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> > DECLARE_PER_CPU(cpumask_var_t, thread_group_l3_cache_map);
> > --
> > 2.31.1
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
2023-10-19 4:48 ` Michael Ellerman
@ 2023-10-19 13:16 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Christophe Leroy, Nicholas Piggin, Peter Zijlstra,
Rohan McLure, Valentin Schneider, Josh Poimboeuf, Mark Rutland,
linux-kernel
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:48:48]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Like L2-cache info, coregroup information which is used to determine MC
> > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > export coregroup information for shared processor LPARs. Hence disable
> > creating MC domains on shared LPAR Systems.
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/smp.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 498c2d51fc20..29da9262cb17 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> >
> > static bool has_coregroup_support(void)
> > {
> > + /* Coregroup identification not available on shared systems */
> > + if (is_shared_processor())
> > + return 0;
>
> That will catch guests running under KVM too right? Do we want that?
>
Only dedicated LPARS on PowerVM expose coregroup or Hemisphere information.
Currently other systems including KVMs don't expose this information to the
OS.
> > return coregroup_enabled;
>
> What does coregroup_enabled mean now?
>
> I'd rather this was actually checking the presence of something, rather
> than just hard coding that shared processor means no coregroup support.
>
On a shared LPAR, the Hypervisors would like to have the flexibility to
schedule LPAR on any core within the DIE without having to further think
about the Hemisphere locality of the core.
> cheers
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
@ 2023-10-19 13:16 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, linux-kernel,
Nicholas Piggin, Rohan McLure, linuxppc-dev, Josh Poimboeuf
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:48:48]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Like L2-cache info, coregroup information which is used to determine MC
> > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > export coregroup information for shared processor LPARs. Hence disable
> > creating MC domains on shared LPAR Systems.
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/smp.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 498c2d51fc20..29da9262cb17 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> >
> > static bool has_coregroup_support(void)
> > {
> > + /* Coregroup identification not available on shared systems */
> > + if (is_shared_processor())
> > + return 0;
>
> That will catch guests running under KVM too right? Do we want that?
>
Only dedicated LPARS on PowerVM expose coregroup or Hemisphere information.
Currently other systems including KVMs don't expose this information to the
OS.
> > return coregroup_enabled;
>
> What does coregroup_enabled mean now?
>
> I'd rather this was actually checking the presence of something, rather
> than just hard coding that shared processor means no coregroup support.
>
On a shared LPAR, the Hypervisors would like to have the flexibility to
schedule LPAR on any core within the DIE without having to further think
about the Hemisphere locality of the core.
> cheers
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
2023-10-19 7:50 ` Peter Zijlstra
@ 2023-10-19 13:23 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Ellerman, linuxppc-dev, Christophe Leroy,
Nicholas Piggin, Rohan McLure, Valentin Schneider,
Josh Poimboeuf, Mark Rutland, linux-kernel
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:50:46]:
> On Thu, Oct 19, 2023 at 03:48:48PM +1100, Michael Ellerman wrote:
> > Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > > Like L2-cache info, coregroup information which is used to determine MC
> > > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > > export coregroup information for shared processor LPARs. Hence disable
> > > creating MC domains on shared LPAR Systems.
> > >
> > > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > > ---
> > > arch/powerpc/kernel/smp.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > > index 498c2d51fc20..29da9262cb17 100644
> > > --- a/arch/powerpc/kernel/smp.c
> > > +++ b/arch/powerpc/kernel/smp.c
> > > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> > >
> > > static bool has_coregroup_support(void)
> > > {
> > > + /* Coregroup identification not available on shared systems */
> > > + if (is_shared_processor())
> > > + return 0;
> >
> > That will catch guests running under KVM too right? Do we want that?
>
> Some KVM people use vcpu pinning and pass-through topology things,
> slice-of-hardware or something like that. In that scenario you actively
> do want this.
>
> I'm fairly clueless on when this is_shared_processor() gets to be true,
> so that might already be dealt with..
Dedicated LPARs are like pinned VM instances. There is a one-to-one mapping
between the Physical core and the virtual core.
On Shared LPARs are unlike regular unpinned VM instances. The Hypervisor
has the flexibility to schedule the virtual core on different Physical core
based on the availability. And like regular VMs, Shared LPARs also support
overcommiting i.e there can be more virtual cores than Physical cores. So
Hypervisor takes care of switching the cores based on its entitlement.
Hence On Shared LPARs, Hypervisors would like to keep the Hemisphere
locality of the cores transparent to the OS.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor
@ 2023-10-19 13:23 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-19 13:23 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Valentin Schneider, linux-kernel, Nicholas Piggin,
Rohan McLure, linuxppc-dev, Josh Poimboeuf
* Peter Zijlstra <peterz@infradead.org> [2023-10-19 09:50:46]:
> On Thu, Oct 19, 2023 at 03:48:48PM +1100, Michael Ellerman wrote:
> > Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > > Like L2-cache info, coregroup information which is used to determine MC
> > > sched domains is only present on dedicated LPARs. i.e PowerVM doesn't
> > > export coregroup information for shared processor LPARs. Hence disable
> > > creating MC domains on shared LPAR Systems.
> > >
> > > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > > ---
> > > arch/powerpc/kernel/smp.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > > index 498c2d51fc20..29da9262cb17 100644
> > > --- a/arch/powerpc/kernel/smp.c
> > > +++ b/arch/powerpc/kernel/smp.c
> > > @@ -1046,6 +1046,10 @@ static struct cpumask *cpu_coregroup_mask(int cpu)
> > >
> > > static bool has_coregroup_support(void)
> > > {
> > > + /* Coregroup identification not available on shared systems */
> > > + if (is_shared_processor())
> > > + return 0;
> >
> > That will catch guests running under KVM too right? Do we want that?
>
> Some KVM people use vcpu pinning and pass-through topology things,
> slice-of-hardware or something like that. In that scenario you actively
> do want this.
>
> I'm fairly clueless on when this is_shared_processor() gets to be true,
> so that might already be dealt with..
Dedicated LPARs are like pinned VM instances. There is a one-to-one mapping
between the Physical core and the virtual core.
On Shared LPARs are unlike regular unpinned VM instances. The Hypervisor
has the flexibility to schedule the virtual core on different Physical core
based on the availability. And like regular VMs, Shared LPARs also support
overcommiting i.e there can be more virtual cores than Physical cores. So
Hypervisor takes care of switching the cores based on its entitlement.
Hence On Shared LPARs, Hypervisors would like to keep the Hemisphere
locality of the cores transparent to the OS.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-18 16:37 ` Srikar Dronamraju
@ 2023-10-19 15:56 ` Shrikanth Hegde
-1 siblings, 0 replies; 53+ messages in thread
From: Shrikanth Hegde @ 2023-10-19 15:56 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, linux-kernel,
Rohan McLure, Nicholas Piggin, linuxppc-dev, Josh Poimboeuf,
Michael Ellerman
On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> If there are shared processor LPARs, underlying Hypervisor can have more
> virtual cores to handle than actual physical cores.
>
> Starting with Power 9, a core has 2 nearly independent thread groups.
> On a shared processors LPARs, it helps to pack threads to lesser number
> of cores so that the overall system performance and utilization
> improves. PowerVM schedules at a core level. Hence packing to fewer
> cores helps.
>
> For example: Lets says there are two 8-core Shared LPARs that are
> actually sharing a 8 Core shared physical pool, each running 8 threads
> each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> them to perform better. This is because each of the LPAR will get
> 100% time to run applications and there will no switching required by
> the Hypervisor.
>
> To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
This would have a conflict with tip/master.
DIE has been renamed to PKG and Both changelog and code below should
change DIE to PKG.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2: Using static key instead of a variable.
>
> arch/powerpc/kernel/smp.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 37c41297c9ce..498c2d51fc20 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> */
> static int powerpc_shared_cache_flags(void)
> {
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> +
> return SD_SHARE_PKG_RESOURCES;
> }
>
> +static int powerpc_shared_proc_flags(void)
> +{
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_ASYM_PACKING;
> +
> + return 0;
> +}
> +
> /*
> * We can't just pass cpu_l2_cache_mask() directly because
> * returns a non-const pointer and the compiler barfs on that.
> @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> #endif
> { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> - { cpu_mc_mask, SD_INIT_NAME(MC) },
> - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> { NULL, },
> };
>
> @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> pr_info_once("Enabling Asymmetric SMT scheduling\n");
> static_branch_enable(&powerpc_asym_packing);
> + } else if (is_shared_processor() && has_big_cores) {
> + static_branch_enable(&powerpc_asym_packing);
> }
>
> #ifdef CONFIG_SCHED_SMT
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-19 15:56 ` Shrikanth Hegde
0 siblings, 0 replies; 53+ messages in thread
From: Shrikanth Hegde @ 2023-10-19 15:56 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, linux-kernel,
Nicholas Piggin, Rohan McLure, linuxppc-dev, Josh Poimboeuf
On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> If there are shared processor LPARs, underlying Hypervisor can have more
> virtual cores to handle than actual physical cores.
>
> Starting with Power 9, a core has 2 nearly independent thread groups.
> On a shared processors LPARs, it helps to pack threads to lesser number
> of cores so that the overall system performance and utilization
> improves. PowerVM schedules at a core level. Hence packing to fewer
> cores helps.
>
> For example: Lets says there are two 8-core Shared LPARs that are
> actually sharing a 8 Core shared physical pool, each running 8 threads
> each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> them to perform better. This is because each of the LPAR will get
> 100% time to run applications and there will no switching required by
> the Hypervisor.
>
> To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
This would have a conflict with tip/master.
DIE has been renamed to PKG and Both changelog and code below should
change DIE to PKG.
>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2: Using static key instead of a variable.
>
> arch/powerpc/kernel/smp.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 37c41297c9ce..498c2d51fc20 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1009,9 +1009,20 @@ static int powerpc_smt_flags(void)
> */
> static int powerpc_shared_cache_flags(void)
> {
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> +
> return SD_SHARE_PKG_RESOURCES;
> }
>
> +static int powerpc_shared_proc_flags(void)
> +{
> + if (static_branch_unlikely(&powerpc_asym_packing))
> + return SD_ASYM_PACKING;
> +
> + return 0;
> +}
> +
> /*
> * We can't just pass cpu_l2_cache_mask() directly because
> * returns a non-const pointer and the compiler barfs on that.
> @@ -1048,8 +1059,8 @@ static struct sched_domain_topology_level powerpc_topology[] = {
> { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) },
> #endif
> { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) },
> - { cpu_mc_mask, SD_INIT_NAME(MC) },
> - { cpu_cpu_mask, SD_INIT_NAME(DIE) },
> + { cpu_mc_mask, powerpc_shared_proc_flags, SD_INIT_NAME(MC) },
> + { cpu_cpu_mask, powerpc_shared_proc_flags, SD_INIT_NAME(DIE) },
> { NULL, },
> };
>
> @@ -1687,6 +1698,8 @@ static void __init fixup_topology(void)
> if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> pr_info_once("Enabling Asymmetric SMT scheduling\n");
> static_branch_enable(&powerpc_asym_packing);
> + } else if (is_shared_processor() && has_big_cores) {
> + static_branch_enable(&powerpc_asym_packing);
> }
>
> #ifdef CONFIG_SCHED_SMT
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
2023-10-19 15:56 ` Shrikanth Hegde
@ 2023-10-20 5:44 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-20 5:44 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, linux-kernel,
Rohan McLure, Nicholas Piggin, linuxppc-dev, Josh Poimboeuf,
Michael Ellerman
* Shrikanth Hegde <sshegde@linux.vnet.ibm.com> [2023-10-19 21:26:56]:
>
>
> On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> > If there are shared processor LPARs, underlying Hypervisor can have more
> > virtual cores to handle than actual physical cores.
> >
> > Starting with Power 9, a core has 2 nearly independent thread groups.
> > On a shared processors LPARs, it helps to pack threads to lesser number
> > of cores so that the overall system performance and utilization
> > improves. PowerVM schedules at a core level. Hence packing to fewer
> > cores helps.
> >
> > For example: Lets says there are two 8-core Shared LPARs that are
> > actually sharing a 8 Core shared physical pool, each running 8 threads
> > each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> > them to perform better. This is because each of the LPAR will get
> > 100% time to run applications and there will no switching required by
> > the Hypervisor.
> >
> > To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
>
> This would have a conflict with tip/master.
> DIE has been renamed to PKG and Both changelog and code below should
> change DIE to PKG.
Once the changes are part of powerpc/merge, will rebase and accomodate the
changes from DIE to PKG.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor
@ 2023-10-20 5:44 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-20 5:44 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, linux-kernel,
Nicholas Piggin, Rohan McLure, linuxppc-dev, Josh Poimboeuf
* Shrikanth Hegde <sshegde@linux.vnet.ibm.com> [2023-10-19 21:26:56]:
>
>
> On 10/18/23 10:07 PM, Srikar Dronamraju wrote:
> > If there are shared processor LPARs, underlying Hypervisor can have more
> > virtual cores to handle than actual physical cores.
> >
> > Starting with Power 9, a core has 2 nearly independent thread groups.
> > On a shared processors LPARs, it helps to pack threads to lesser number
> > of cores so that the overall system performance and utilization
> > improves. PowerVM schedules at a core level. Hence packing to fewer
> > cores helps.
> >
> > For example: Lets says there are two 8-core Shared LPARs that are
> > actually sharing a 8 Core shared physical pool, each running 8 threads
> > each. Then Consolidating 8 threads to 4 cores on each LPAR would help
> > them to perform better. This is because each of the LPAR will get
> > 100% time to run applications and there will no switching required by
> > the Hypervisor.
> >
> > To achieve this, enable SD_ASYM_PACKING flag at CACHE, MC and DIE level.
>
> This would have a conflict with tip/master.
> DIE has been renamed to PKG and Both changelog and code below should
> change DIE to PKG.
Once the changes are part of powerpc/merge, will rebase and accomodate the
changes from DIE to PKG.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
2023-10-19 4:33 ` Michael Ellerman
@ 2023-10-20 9:09 ` Srikar Dronamraju
-1 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-20 9:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Christophe Leroy, Nicholas Piggin, Peter Zijlstra,
Rohan McLure, Valentin Schneider, Josh Poimboeuf, ndesaulniers,
Mark Rutland, linux-kernel
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:33:16]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Currently cpu feature flag is checked whenever powerpc_smt_flags gets
> > called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
> > on the processor and all processors will either have this set or will
> > have it unset.
>
> The cpu_has_feature() test is implemented with a static key.
>
> So AFAICS this is just replacing one static key with another?
>
> I see that you use the new static key in subsequent patches. But
> couldn't those just use the existing cpu feature test?
>
Yes, we can use the existing cpu feature test itself.
> Anyway I'd be interested to see how the generated code differs
> before/after this.
>
---------------------------->8----------------------------------------------8<------------
Before this change
0000000000000500 <powerpc_smt_flags>:
{
500: 00 00 4c 3c addis r2,r12,0
504: 00 00 42 38 addi r2,r2,0
508: a6 02 08 7c mflr r0
50c: 01 00 00 48 bl 50c <powerpc_smt_flags+0xc>
510: f8 ff e1 fb std r31,-8(r1)
514: 91 ff 21 f8 stdu r1,-112(r1)
#define JUMP_ENTRY_TYPE stringify_in_c(FTR_ENTRY_LONG)
#define JUMP_LABEL_NOP_SIZE 4
static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
asm_volatile_goto("1:\n\t"
518: 00 00 00 60 nop
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
51c: 00 00 22 3d addis r9,r2,0
flags |= SD_ASYM_PACKING;
520: 80 05 e0 3b li r31,1408
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
524: 00 00 29 89 lbz r9,0(r9)
528: 00 00 09 2c cmpwi r9,0
52c: 28 00 82 41 beq 554 <powerpc_smt_flags+0x54>
}
530: 70 00 21 38 addi r1,r1,112
534: b4 07 e3 7f extsw r3,r31
538: f8 ff e1 eb ld r31,-8(r1)
53c: 20 00 80 4e blr
int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
540: 80 01 e0 3b li r31,384
}
544: 70 00 21 38 addi r1,r1,112
548: b4 07 e3 7f extsw r3,r31
54c: f8 ff e1 eb ld r31,-8(r1)
550: 20 00 80 4e blr
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
554: a6 02 08 7c mflr r0
558: 00 00 62 3c addis r3,r2,0
55c: 01 00 20 39 li r9,1
560: 00 00 42 3d addis r10,r2,0
564: 00 00 63 38 addi r3,r3,0
568: 00 00 2a 99 stb r9,0(r10)
56c: 80 00 01 f8 std r0,128(r1)
570: 01 00 00 48 bl 570 <powerpc_smt_flags+0x70>
574: 00 00 00 60 nop
578: 80 00 01 e8 ld r0,128(r1)
57c: a6 03 08 7c mtlr r0
580: b0 ff ff 4b b 530 <powerpc_smt_flags+0x30>
584: 00 00 00 60 nop
588: 00 00 00 60 nop
58c: 00 00 00 60 nop
post this change.
0000000000000340 <powerpc_smt_flags>:
{
340: a6 02 08 7c mflr r0
344: 01 00 00 48 bl 344 <powerpc_smt_flags+0x4>
#define JUMP_ENTRY_TYPE stringify_in_c(FTR_ENTRY_LONG)
#define JUMP_LABEL_NOP_SIZE 4
static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
asm_volatile_goto("1:\n\t"
348: 00 00 00 60 nop
return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
34c: 80 01 60 38 li r3,384
}
350: b4 07 63 7c extsw r3,r3
354: 20 00 80 4e blr
358: 00 00 00 60 nop
35c: 00 00 00 60 nop
return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
360: 80 05 60 38 li r3,1408
}
364: b4 07 63 7c extsw r3,r3
368: 20 00 80 4e blr
36c: 00 00 00 60 nop
---------------------------->8----------------------------------------------8<------------
I think the most of the difference is due to moving pr_info_once to
fixup_topology. Does it make sense to move the pr_info_once to
fixup_topology (which is called less often) from powerpc_smt_flags?
Even though the pr_info_once would probably translate to load + cmp + branch
we could avoid that for each smt_flag call.
So something like
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5826f5108a12..bc22f775425b 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -993,13 +993,10 @@ static bool shared_caches;
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
- if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
- printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
- flags |= SD_ASYM_PACKING;
- }
- return flags;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
}
#endif
@@ -1687,6 +1684,9 @@ static void __init fixup_topology(void)
int i;
#ifdef CONFIG_SCHED_SMT
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT))
+ pr_info_once("Enabling Asymmetric SMT scheduling\n");
+
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
powerpc_topology[smt_idx].mask = smallcore_smt_mask;
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP
@ 2023-10-20 9:09 ` Srikar Dronamraju
0 siblings, 0 replies; 53+ messages in thread
From: Srikar Dronamraju @ 2023-10-20 9:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mark Rutland, Valentin Schneider, Peter Zijlstra, ndesaulniers,
linux-kernel, Nicholas Piggin, Rohan McLure, linuxppc-dev,
Josh Poimboeuf
* Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:33:16]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > Currently cpu feature flag is checked whenever powerpc_smt_flags gets
> > called. This is an unnecessary overhead. CPU_FTR_ASYM_SMT is set based
> > on the processor and all processors will either have this set or will
> > have it unset.
>
> The cpu_has_feature() test is implemented with a static key.
>
> So AFAICS this is just replacing one static key with another?
>
> I see that you use the new static key in subsequent patches. But
> couldn't those just use the existing cpu feature test?
>
Yes, we can use the existing cpu feature test itself.
> Anyway I'd be interested to see how the generated code differs
> before/after this.
>
---------------------------->8----------------------------------------------8<------------
Before this change
0000000000000500 <powerpc_smt_flags>:
{
500: 00 00 4c 3c addis r2,r12,0
504: 00 00 42 38 addi r2,r2,0
508: a6 02 08 7c mflr r0
50c: 01 00 00 48 bl 50c <powerpc_smt_flags+0xc>
510: f8 ff e1 fb std r31,-8(r1)
514: 91 ff 21 f8 stdu r1,-112(r1)
#define JUMP_ENTRY_TYPE stringify_in_c(FTR_ENTRY_LONG)
#define JUMP_LABEL_NOP_SIZE 4
static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
asm_volatile_goto("1:\n\t"
518: 00 00 00 60 nop
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
51c: 00 00 22 3d addis r9,r2,0
flags |= SD_ASYM_PACKING;
520: 80 05 e0 3b li r31,1408
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
524: 00 00 29 89 lbz r9,0(r9)
528: 00 00 09 2c cmpwi r9,0
52c: 28 00 82 41 beq 554 <powerpc_smt_flags+0x54>
}
530: 70 00 21 38 addi r1,r1,112
534: b4 07 e3 7f extsw r3,r31
538: f8 ff e1 eb ld r31,-8(r1)
53c: 20 00 80 4e blr
int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
540: 80 01 e0 3b li r31,384
}
544: 70 00 21 38 addi r1,r1,112
548: b4 07 e3 7f extsw r3,r31
54c: f8 ff e1 eb ld r31,-8(r1)
550: 20 00 80 4e blr
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
554: a6 02 08 7c mflr r0
558: 00 00 62 3c addis r3,r2,0
55c: 01 00 20 39 li r9,1
560: 00 00 42 3d addis r10,r2,0
564: 00 00 63 38 addi r3,r3,0
568: 00 00 2a 99 stb r9,0(r10)
56c: 80 00 01 f8 std r0,128(r1)
570: 01 00 00 48 bl 570 <powerpc_smt_flags+0x70>
574: 00 00 00 60 nop
578: 80 00 01 e8 ld r0,128(r1)
57c: a6 03 08 7c mtlr r0
580: b0 ff ff 4b b 530 <powerpc_smt_flags+0x30>
584: 00 00 00 60 nop
588: 00 00 00 60 nop
58c: 00 00 00 60 nop
post this change.
0000000000000340 <powerpc_smt_flags>:
{
340: a6 02 08 7c mflr r0
344: 01 00 00 48 bl 344 <powerpc_smt_flags+0x4>
#define JUMP_ENTRY_TYPE stringify_in_c(FTR_ENTRY_LONG)
#define JUMP_LABEL_NOP_SIZE 4
static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
asm_volatile_goto("1:\n\t"
348: 00 00 00 60 nop
return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
34c: 80 01 60 38 li r3,384
}
350: b4 07 63 7c extsw r3,r3
354: 20 00 80 4e blr
358: 00 00 00 60 nop
35c: 00 00 00 60 nop
return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
360: 80 05 60 38 li r3,1408
}
364: b4 07 63 7c extsw r3,r3
368: 20 00 80 4e blr
36c: 00 00 00 60 nop
---------------------------->8----------------------------------------------8<------------
I think the most of the difference is due to moving pr_info_once to
fixup_topology. Does it make sense to move the pr_info_once to
fixup_topology (which is called less often) from powerpc_smt_flags?
Even though the pr_info_once would probably translate to load + cmp + branch
we could avoid that for each smt_flag call.
So something like
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5826f5108a12..bc22f775425b 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -993,13 +993,10 @@ static bool shared_caches;
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
- if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
- printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
- flags |= SD_ASYM_PACKING;
- }
- return flags;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
}
#endif
@@ -1687,6 +1684,9 @@ static void __init fixup_topology(void)
int i;
#ifdef CONFIG_SCHED_SMT
+ if (cpu_has_feature(CPU_FTR_ASYM_SMT))
+ pr_info_once("Enabling Asymmetric SMT scheduling\n");
+
if (has_big_cores) {
pr_info("Big cores detected but using small core scheduling\n");
powerpc_topology[smt_idx].mask = smallcore_smt_mask;
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
2023-10-19 13:08 ` Srikar Dronamraju
(?)
@ 2023-10-20 10:44 ` Michael Ellerman
-1 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-20 10:44 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Peter Zijlstra, linux-kernel, x86,
Ajay Kaher, Nicholas Piggin, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Alexey Makhalov,
linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> * Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:41:40]:
>
>> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > The ability to detect if the system is running in a shared processor
>> > mode is helpful in few more generic cases not just in
>> > paravirtualization.
>> > For example: At boot time, different scheduler/ topology flags may be
>> > set based on the processor mode. Hence move it to a more generic file.
>>
>> I'd rather you just included paravirt.h in the few files where you need it.
>
> I thought, detecting if a Processor was shared or not was more a
> smp/processor related than a paravirt related.
It's both really :)
It's definitely paravirt related though, because if we weren't
*para*virt then we wouldn't know there was a hypervisor at all :)
But having smaller more focused headers is preferable in general just
for mechanical reasons.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-20 10:44 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-20 10:44 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Valentin Schneider, Peter Zijlstra, linux-kernel, x86,
Ajay Kaher, Nicholas Piggin, virtualization,
VMware PV-Drivers Reviewers, Rohan McLure, Christophe Leroy,
Alexey Makhalov, linuxppc-dev, Josh Poimboeuf
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> * Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:41:40]:
>
>> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > The ability to detect if the system is running in a shared processor
>> > mode is helpful in few more generic cases not just in
>> > paravirtualization.
>> > For example: At boot time, different scheduler/ topology flags may be
>> > set based on the processor mode. Hence move it to a more generic file.
>>
>> I'd rather you just included paravirt.h in the few files where you need it.
>
> I thought, detecting if a Processor was shared or not was more a
> smp/processor related than a paravirt related.
It's both really :)
It's definitely paravirt related though, because if we weren't
*para*virt then we wouldn't know there was a hypervisor at all :)
But having smaller more focused headers is preferable in general just
for mechanical reasons.
cheers
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h
@ 2023-10-20 10:44 ` Michael Ellerman
0 siblings, 0 replies; 53+ messages in thread
From: Michael Ellerman @ 2023-10-20 10:44 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: linuxppc-dev, Christophe Leroy, Nicholas Piggin, Peter Zijlstra,
Rohan McLure, Valentin Schneider, Ajay Kaher, Alexey Makhalov,
VMware PV-Drivers Reviewers, Josh Poimboeuf, virtualization, x86,
linux-kernel
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> * Michael Ellerman <mpe@ellerman.id.au> [2023-10-19 15:41:40]:
>
>> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
>> > The ability to detect if the system is running in a shared processor
>> > mode is helpful in few more generic cases not just in
>> > paravirtualization.
>> > For example: At boot time, different scheduler/ topology flags may be
>> > set based on the processor mode. Hence move it to a more generic file.
>>
>> I'd rather you just included paravirt.h in the few files where you need it.
>
> I thought, detecting if a Processor was shared or not was more a
> smp/processor related than a paravirt related.
It's both really :)
It's definitely paravirt related though, because if we weren't
*para*virt then we wouldn't know there was a hypervisor at all :)
But having smaller more focused headers is preferable in general just
for mechanical reasons.
cheers
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2023-10-20 10:45 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 16:37 [PATCH v2 0/6] powerpc/smp: Shared processor sched optimizations Srikar Dronamraju
2023-10-18 16:37 ` [PATCH v2 1/6] powerpc/smp: Cache CPU has Asymmetric SMP Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
2023-10-19 4:33 ` Michael Ellerman
2023-10-19 4:33 ` Michael Ellerman
2023-10-20 9:09 ` Srikar Dronamraju
2023-10-20 9:09 ` Srikar Dronamraju
2023-10-18 16:37 ` [PATCH v2 2/6] powerpc/smp: Enable Asym packing for cores on shared processor Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
2023-10-19 4:38 ` Michael Ellerman
2023-10-19 4:38 ` Michael Ellerman
2023-10-19 7:48 ` Peter Zijlstra
2023-10-19 7:48 ` Peter Zijlstra
2023-10-19 11:50 ` Michael Ellerman
2023-10-19 11:50 ` Michael Ellerman
2023-10-19 12:54 ` Srikar Dronamraju
2023-10-19 12:54 ` Srikar Dronamraju
2023-10-19 15:56 ` Shrikanth Hegde
2023-10-19 15:56 ` Shrikanth Hegde
2023-10-20 5:44 ` Srikar Dronamraju
2023-10-20 5:44 ` Srikar Dronamraju
2023-10-18 16:37 ` [PATCH v2 3/6] powerpc/smp: Move shared_processor static key to smp.h Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
2023-10-19 4:41 ` Michael Ellerman
2023-10-19 4:41 ` Michael Ellerman
2023-10-19 4:41 ` Michael Ellerman
2023-10-19 13:08 ` Srikar Dronamraju
2023-10-19 13:08 ` Srikar Dronamraju
2023-10-20 10:44 ` Michael Ellerman
2023-10-20 10:44 ` Michael Ellerman
2023-10-20 10:44 ` Michael Ellerman
2023-10-19 10:30 ` Shrikanth Hegde
2023-10-19 10:30 ` Shrikanth Hegde
2023-10-18 16:37 ` [PATCH v2 4/6] powerpc/smp: Disable MC domain for shared processor Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
2023-10-19 4:48 ` Michael Ellerman
2023-10-19 4:48 ` Michael Ellerman
2023-10-19 7:50 ` Peter Zijlstra
2023-10-19 7:50 ` Peter Zijlstra
2023-10-19 13:23 ` Srikar Dronamraju
2023-10-19 13:23 ` Srikar Dronamraju
2023-10-19 13:16 ` Srikar Dronamraju
2023-10-19 13:16 ` Srikar Dronamraju
2023-10-18 16:37 ` [PATCH v2 5/6] powerpc/smp: Add read_mostly attribute Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
2023-10-19 4:49 ` Michael Ellerman
2023-10-19 4:49 ` Michael Ellerman
2023-10-19 7:51 ` Peter Zijlstra
2023-10-19 7:51 ` Peter Zijlstra
2023-10-19 12:56 ` Srikar Dronamraju
2023-10-19 12:56 ` Srikar Dronamraju
2023-10-18 16:37 ` [PATCH v2 6/6] powerpc/smp: Avoid asym packing within thread_group of a core Srikar Dronamraju
2023-10-18 16:37 ` Srikar Dronamraju
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.