To associate the SD flags with some metadata, we need some more structure
in the way they are declared.
Rather than shove that in a free-standing macro list, move the declaration
in a separate file that can be re-imported with different SD_FLAG
definitions. This is inspired by the syscalls numbering header.
No change in functionality.
Signed-off-by: Valentin Schneider <[email protected]>
---
include/linux/sched/sd_flags.h | 33 +++++++++++++++++++++++++++++++++
include/linux/sched/topology.h | 17 +++--------------
2 files changed, 36 insertions(+), 14 deletions(-)
create mode 100644 include/linux/sched/sd_flags.h
diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
new file mode 100644
index 000000000000..685bbe736945
--- /dev/null
+++ b/include/linux/sched/sd_flags.h
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * sched-domains (multiprocessor balancing) flag declarations.
+ */
+
+/* Balance when about to become idle */
+SD_FLAG(SD_BALANCE_NEWIDLE, 0)
+/* Balance on exec */
+SD_FLAG(SD_BALANCE_EXEC, 1)
+/* Balance on fork, clone */
+SD_FLAG(SD_BALANCE_FORK, 2)
+/* Balance on wakeup */
+SD_FLAG(SD_BALANCE_WAKE, 3)
+/* Wake task to waking CPU */
+SD_FLAG(SD_WAKE_AFFINE, 4)
+/* Domain members have different CPU capacities */
+SD_FLAG(SD_ASYM_CPUCAPACITY, 5)
+/* Domain members share CPU capacity */
+SD_FLAG(SD_SHARE_CPUCAPACITY, 6)
+/* Domain members share power domain */
+SD_FLAG(SD_SHARE_POWERDOMAIN, 7)
+/* Domain members share CPU pkg resources */
+SD_FLAG(SD_SHARE_PKG_RESOURCES, 8)
+/* Only a single load balancing instance */
+SD_FLAG(SD_SERIALIZE, 9)
+/* Place busy groups earlier in the domain */
+SD_FLAG(SD_ASYM_PACKING, 10)
+/* Prefer to place tasks in a sibling domain */
+SD_FLAG(SD_PREFER_SIBLING, 11)
+/* sched_domains of this level overlap */
+SD_FLAG(SD_OVERLAP, 12)
+/* cross-node balancing */
+SD_FLAG(SD_NUMA, 13)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index 8de2f9744569..db7d24c0174b 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -11,20 +11,9 @@
*/
#ifdef CONFIG_SMP
-#define SD_BALANCE_NEWIDLE 0x0001 /* Balance when about to become idle */
-#define SD_BALANCE_EXEC 0x0002 /* Balance on exec */
-#define SD_BALANCE_FORK 0x0004 /* Balance on fork, clone */
-#define SD_BALANCE_WAKE 0x0008 /* Balance on wakeup */
-#define SD_WAKE_AFFINE 0x0010 /* Wake task to waking CPU */
-#define SD_ASYM_CPUCAPACITY 0x0020 /* Domain members have different CPU capacities */
-#define SD_SHARE_CPUCAPACITY 0x0040 /* Domain members share CPU capacity */
-#define SD_SHARE_POWERDOMAIN 0x0080 /* Domain members share power domain */
-#define SD_SHARE_PKG_RESOURCES 0x0100 /* Domain members share CPU pkg resources */
-#define SD_SERIALIZE 0x0200 /* Only a single load balancing instance */
-#define SD_ASYM_PACKING 0x0400 /* Place busy groups earlier in the domain */
-#define SD_PREFER_SIBLING 0x0800 /* Prefer to place tasks in a sibling domain */
-#define SD_OVERLAP 0x1000 /* sched_domains of this level overlap */
-#define SD_NUMA 0x2000 /* cross-node balancing */
+#define SD_FLAG(name, idx) static const unsigned int name = BIT(idx);
+#include <linux/sched/sd_flags.h>
+#undef SD_FLAG
#ifdef CONFIG_SCHED_SMT
static inline int cpu_smt_flags(void)
--
2.24.0
On Wed, Mar 11, 2020 at 06:33:18PM +0000, Valentin Schneider wrote:
> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
> new file mode 100644
> index 000000000000..685bbe736945
> --- /dev/null
> +++ b/include/linux/sched/sd_flags.h
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * sched-domains (multiprocessor balancing) flag declarations.
> + */
> +
> +/* Balance when about to become idle */
> +SD_FLAG(SD_BALANCE_NEWIDLE, 0)
> +/* Balance on exec */
> +SD_FLAG(SD_BALANCE_EXEC, 1)
> +/* Balance on fork, clone */
> +SD_FLAG(SD_BALANCE_FORK, 2)
> +/* Balance on wakeup */
> +SD_FLAG(SD_BALANCE_WAKE, 3)
> +/* Wake task to waking CPU */
> +SD_FLAG(SD_WAKE_AFFINE, 4)
Isn't it more like: "Consider waking task on waking CPU"?
IIRC, with this flag set the wake-up can happen either near prev_cpu or
this_cpu.
> +/* Domain members have different CPU capacities */
> +SD_FLAG(SD_ASYM_CPUCAPACITY, 5)
> +/* Domain members share CPU capacity */
> +SD_FLAG(SD_SHARE_CPUCAPACITY, 6)
Perhaps add +" (SMT)" to the comment to help the uninitiated
understanding it a bit easier?
> +/* Domain members share power domain */
> +SD_FLAG(SD_SHARE_POWERDOMAIN, 7)
This flag is set only by 32-bit arm and has never had any effect. I
think it was the beginning of something years ago that hasn't
progressed. Perhaps we can remove it now?
> +/* Domain members share CPU pkg resources */
> +SD_FLAG(SD_SHARE_PKG_RESOURCES, 8)
+" (e.g. caches)" ?
> +/* Only a single load balancing instance */
> +SD_FLAG(SD_SERIALIZE, 9)
> +/* Place busy groups earlier in the domain */
> +SD_FLAG(SD_ASYM_PACKING, 10)
Place busy _tasks_ earlier in the domain?
It is a bit unclear what 'earlier' means here but since the packing
ordering can actually be defined by the architecture, we can't be much
more specific I guess.
Morten
Hi Morten,
Just as a heads-up, I think those changes would better fit 2/3, or be
in their own patch. 1/3 is just a straight up code move, with no changes
to the existing comments.
On Mon, Mar 23 2020, Morten Rasmussen wrote:
> On Wed, Mar 11, 2020 at 06:33:18PM +0000, Valentin Schneider wrote:
>> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
>> new file mode 100644
>> index 000000000000..685bbe736945
>> --- /dev/null
>> +++ b/include/linux/sched/sd_flags.h
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * sched-domains (multiprocessor balancing) flag declarations.
>> + */
>> +
>> +/* Balance when about to become idle */
>> +SD_FLAG(SD_BALANCE_NEWIDLE, 0)
>> +/* Balance on exec */
>> +SD_FLAG(SD_BALANCE_EXEC, 1)
>> +/* Balance on fork, clone */
>> +SD_FLAG(SD_BALANCE_FORK, 2)
>> +/* Balance on wakeup */
>> +SD_FLAG(SD_BALANCE_WAKE, 3)
>> +/* Wake task to waking CPU */
>> +SD_FLAG(SD_WAKE_AFFINE, 4)
>
> Isn't it more like: "Consider waking task on waking CPU"?
>
> IIRC, with this flag set the wake-up can happen either near prev_cpu or
> this_cpu.
>
Right, it's not a hard guarantee.
>> +/* Domain members have different CPU capacities */
>> +SD_FLAG(SD_ASYM_CPUCAPACITY, 5)
>> +/* Domain members share CPU capacity */
>> +SD_FLAG(SD_SHARE_CPUCAPACITY, 6)
>
> Perhaps add +" (SMT)" to the comment to help the uninitiated
> understanding it a bit easier?
>
Sounds good.
>> +/* Domain members share power domain */
>> +SD_FLAG(SD_SHARE_POWERDOMAIN, 7)
>
> This flag is set only by 32-bit arm and has never had any effect. I
> think it was the beginning of something years ago that hasn't
> progressed. Perhaps we can remove it now?
>
Right, I don't think I've seen anything recent that uses that flag.
>> +/* Domain members share CPU pkg resources */
>> +SD_FLAG(SD_SHARE_PKG_RESOURCES, 8)
>
> +" (e.g. caches)" ?
>
Agreed! I actually already have that one in 2/3.
>> +/* Only a single load balancing instance */
>> +SD_FLAG(SD_SERIALIZE, 9)
>> +/* Place busy groups earlier in the domain */
>> +SD_FLAG(SD_ASYM_PACKING, 10)
>
> Place busy _tasks_ earlier in the domain?
>
Ack.
> It is a bit unclear what 'earlier' means here but since the packing
> ordering can actually be defined by the architecture, we can't be much
> more specific I guess.
>
This probably dates back to when ASYM_PACKING was really just for
bubbling tasks up to the first CPU of each core, and hasn't been changed
when the asym_priority thing was introduced. I can add a pointer to that.
> Morten
On Mon, Mar 23, 2020 at 05:10:00PM +0000, Valentin Schneider wrote:
>
> Hi Morten,
>
> Just as a heads-up, I think those changes would better fit 2/3, or be
> in their own patch. 1/3 is just a straight up code move, with no changes
> to the existing comments.
I realized that when I got to 2/3, and yes, most of the comments are not
really related to your proposal it more things we could fix while we are
touching that code anyway.
> On Mon, Mar 23 2020, Morten Rasmussen wrote:
> > On Wed, Mar 11, 2020 at 06:33:18PM +0000, Valentin Schneider wrote:
> >> +/* Domain members share power domain */
> >> +SD_FLAG(SD_SHARE_POWERDOMAIN, 7)
> >
> > This flag is set only by 32-bit arm and has never had any effect. I
> > think it was the beginning of something years ago that hasn't
> > progressed. Perhaps we can remove it now?
> >
>
> Right, I don't think I've seen anything recent that uses that flag.
AFAIK, it has never been used, only set.
Morten