The user_cpus_ptr field was originally added by commit b90ca8badbd1
("sched: Introduce task_struct::user_cpus_ptr to track requested
affinity"). It was used only by arm64 arch due to possible asymmetric
CPU setup.
Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested
cpumask"), task_struct::user_cpus_ptr is repurposed to store user
requested cpu affinity specified in the sched_setaffinity().
This results in a slight performance regression on an arm64
system when booted with "allow_mismatched_32bit_el0"
on the command-line. The arch code will (amongst
other things) calls force_compatible_cpus_allowed_ptr() and
relax_compatible_cpus_allowed_ptr() when exec()'ing a 32-bit or a 64-bit
task respectively. Now a call to relax_compatible_cpus_allowed_ptr()
will always result in a __sched_setaffinity() call whether there is a
previous force_compatible_cpus_allowed_ptr() call or not.
In order to fix this regression, a new scheduler flag
task_struct::cpus_allowed_restricted is now added to track if
force_compatible_cpus_allowed_ptr() has been called before or not. This
patch also updates the comments in force_compatible_cpus_allowed_ptr()
and relax_compatible_cpus_allowed_ptr() and handles their interaction
with sched_setaffinity().
This patch also removes the task_user_cpus() helper. In the case of
relax_compatible_cpus_allowed_ptr(), cpu_possible_mask as user_cpu_ptr
masking will be performed within __sched_setaffinity() anyway.
Fixes: 8f9ea86fdf99 ("sched: Always preserve the user requested cpumask")
Reported-by: Will Deacon <[email protected]>
Signed-off-by: Waiman Long <[email protected]>
---
include/linux/sched.h | 3 +++
kernel/sched/core.c | 25 +++++++++++++++++--------
kernel/sched/sched.h | 8 +-------
3 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 853d08f7562b..f93f62a1f858 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -886,6 +886,9 @@ struct task_struct {
unsigned sched_contributes_to_load:1;
unsigned sched_migrated:1;
+ /* restrict_cpus_allowed_ptr() bit, serialized by scheduler locks */
+ unsigned cpus_allowed_restricted:1;
+
/* Force alignment to the next boundary: */
unsigned :0;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bb1ee6d7bdde..d7bc809c109e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2999,6 +2999,10 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,
struct rq *rq;
rq = task_rq_lock(p, &rf);
+
+ if (ctx->flags & SCA_CLR_RESTRICT)
+ p->cpus_allowed_restricted = 0;
+
/*
* Masking should be skipped if SCA_USER or any of the SCA_MIGRATE_*
* flags are set.
@@ -3025,8 +3029,8 @@ EXPORT_SYMBOL_GPL(set_cpus_allowed_ptr);
/*
* Change a given task's CPU affinity to the intersection of its current
* affinity mask and @subset_mask, writing the resulting mask to @new_mask.
- * If user_cpus_ptr is defined, use it as the basis for restricting CPU
- * affinity or use cpu_online_mask instead.
+ * The cpus_allowed_restricted bit is set to indicate to a later
+ * relax_compatible_cpus_allowed_ptr() call to relax the cpumask.
*
* If the resulting mask is empty, leave the affinity unchanged and return
* -EINVAL.
@@ -3044,6 +3048,7 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
int err;
rq = task_rq_lock(p, &rf);
+ p->cpus_allowed_restricted = 1;
/*
* Forcefully restricting the affinity of a deadline task is
@@ -3055,7 +3060,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
goto err_unlock;
}
- if (!cpumask_and(new_mask, task_user_cpus(p), subset_mask)) {
+ if (p->user_cpu_ptr &&
+ !cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) {
err = -EINVAL;
goto err_unlock;
}
@@ -3069,9 +3075,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
/*
* Restrict the CPU affinity of task @p so that it is a subset of
- * task_cpu_possible_mask() and point @p->user_cpus_ptr to a copy of the
- * old affinity mask. If the resulting mask is empty, we warn and walk
- * up the cpuset hierarchy until we find a suitable mask.
+ * task_cpu_possible_mask(). If the resulting mask is empty, we warn
+ * and walk up the cpuset hierarchy until we find a suitable mask.
*/
void force_compatible_cpus_allowed_ptr(struct task_struct *p)
{
@@ -3125,11 +3130,15 @@ __sched_setaffinity(struct task_struct *p, struct affinity_context *ctx);
void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
{
struct affinity_context ac = {
- .new_mask = task_user_cpus(p),
- .flags = 0,
+ .new_mask = cpu_possible_mask;
+ .flags = SCA_CLR_RESTRICT,
};
int ret;
+ /* Return if no previous force_compatible_cpus_allowed_ptr() call */
+ if (!data_race(p->cpus_allowed_restricted))
+ return;
+
/*
* Try to restore the old affinity mask with __sched_setaffinity().
* Cpuset masking will be done there too.
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 771f8ddb7053..e32ba5d9bb54 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1881,13 +1881,6 @@ static inline void dirty_sched_domain_sysctl(int cpu)
#endif
extern int sched_update_scaling(void);
-
-static inline const struct cpumask *task_user_cpus(struct task_struct *p)
-{
- if (!p->user_cpus_ptr)
- return cpu_possible_mask; /* &init_task.cpus_mask */
- return p->user_cpus_ptr;
-}
#endif /* CONFIG_SMP */
#include "stats.h"
@@ -2293,6 +2286,7 @@ extern struct task_struct *pick_next_task_idle(struct rq *rq);
#define SCA_MIGRATE_DISABLE 0x02
#define SCA_MIGRATE_ENABLE 0x04
#define SCA_USER 0x08
+#define SCA_CLR_RESTRICT 0x10
#ifdef CONFIG_SMP
--
2.31.1
On 27.01.23 02:55, Waiman Long wrote:
> The user_cpus_ptr field was originally added by commit b90ca8badbd1
> ("sched: Introduce task_struct::user_cpus_ptr to track requested
> affinity"). It was used only by arm64 arch due to possible asymmetric
> CPU setup.
> [...]> Fixes: 8f9ea86fdf99 ("sched: Always preserve the user requested
cpumask")
> Reported-by: Will Deacon <[email protected]>
Many thx for taking care of this. There is one small thing to improve,
please add the following tag here to make things easier for future code
archaeologists:
Link: https://lore.kernel.org/lkml/20230117160825.GA17756@willie-the-truck/
Maybe also...
Link: https://lore.kernel.org/lkml/20230124194805.GA27257@willie-the-truck/
...as it provides additional info that might be handy at some point.
BTW, Will, thx for CCing me on that.
To explain: Linus[1] and others considered proper link tags with the URL
to the report important in cases like this, as they allow anyone to look
into the backstory of a fix weeks or years later. That's nothing new,
the documentation[2] for some time says to place such tags in cases like
this. I care personally (and made it a bit more explicit in the docs a
while ago), because these tags make my regression tracking efforts a
whole lot easier, as they allow my tracking bot 'regzbot' to
automatically connect reports with patches posted or committed to fix
tracked regressions.
Apropos regzbot, let me tell regzbot to monitor this thread:
#regzbot ^backmonitor:
https://lore.kernel.org/lkml/20230117160825.GA17756@willie-the-truck/
> [...]
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/
[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
Hi Waiman,
On Thu, Jan 26, 2023 at 08:55:27PM -0500, Waiman Long wrote:
> The user_cpus_ptr field was originally added by commit b90ca8badbd1
> ("sched: Introduce task_struct::user_cpus_ptr to track requested
> affinity"). It was used only by arm64 arch due to possible asymmetric
> CPU setup.
>
> Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested
> cpumask"), task_struct::user_cpus_ptr is repurposed to store user
> requested cpu affinity specified in the sched_setaffinity().
>
> This results in a slight performance regression on an arm64
> system when booted with "allow_mismatched_32bit_el0"
> on the command-line. The arch code will (amongst
> other things) calls force_compatible_cpus_allowed_ptr() and
> relax_compatible_cpus_allowed_ptr() when exec()'ing a 32-bit or a 64-bit
> task respectively. Now a call to relax_compatible_cpus_allowed_ptr()
> will always result in a __sched_setaffinity() call whether there is a
> previous force_compatible_cpus_allowed_ptr() call or not.
>
> In order to fix this regression, a new scheduler flag
> task_struct::cpus_allowed_restricted is now added to track if
> force_compatible_cpus_allowed_ptr() has been called before or not. This
> patch also updates the comments in force_compatible_cpus_allowed_ptr()
> and relax_compatible_cpus_allowed_ptr() and handles their interaction
> with sched_setaffinity().
>
> This patch also removes the task_user_cpus() helper. In the case of
> relax_compatible_cpus_allowed_ptr(), cpu_possible_mask as user_cpu_ptr
> masking will be performed within __sched_setaffinity() anyway.
>
> Fixes: 8f9ea86fdf99 ("sched: Always preserve the user requested cpumask")
> Reported-by: Will Deacon <[email protected]>
> Signed-off-by: Waiman Long <[email protected]>
> ---
> include/linux/sched.h | 3 +++
> kernel/sched/core.c | 25 +++++++++++++++++--------
> kernel/sched/sched.h | 8 +-------
> 3 files changed, 21 insertions(+), 15 deletions(-)
So this doesn't even build...
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index bb1ee6d7bdde..d7bc809c109e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2999,6 +2999,10 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,
> struct rq *rq;
>
> rq = task_rq_lock(p, &rf);
> +
> + if (ctx->flags & SCA_CLR_RESTRICT)
> + p->cpus_allowed_restricted = 0;
> +
> /*
> * Masking should be skipped if SCA_USER or any of the SCA_MIGRATE_*
> * flags are set.
> @@ -3025,8 +3029,8 @@ EXPORT_SYMBOL_GPL(set_cpus_allowed_ptr);
> /*
> * Change a given task's CPU affinity to the intersection of its current
> * affinity mask and @subset_mask, writing the resulting mask to @new_mask.
> - * If user_cpus_ptr is defined, use it as the basis for restricting CPU
> - * affinity or use cpu_online_mask instead.
> + * The cpus_allowed_restricted bit is set to indicate to a later
> + * relax_compatible_cpus_allowed_ptr() call to relax the cpumask.
> *
> * If the resulting mask is empty, leave the affinity unchanged and return
> * -EINVAL.
> @@ -3044,6 +3048,7 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
> int err;
>
> rq = task_rq_lock(p, &rf);
> + p->cpus_allowed_restricted = 1;
>
> /*
> * Forcefully restricting the affinity of a deadline task is
> @@ -3055,7 +3060,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
> goto err_unlock;
> }
>
> - if (!cpumask_and(new_mask, task_user_cpus(p), subset_mask)) {
> + if (p->user_cpu_ptr &&
> + !cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) {
s/user_cpu_ptr/user_cpus_ptr/
> err = -EINVAL;
> goto err_unlock;
> }
> @@ -3069,9 +3075,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
>
> /*
> * Restrict the CPU affinity of task @p so that it is a subset of
> - * task_cpu_possible_mask() and point @p->user_cpus_ptr to a copy of the
> - * old affinity mask. If the resulting mask is empty, we warn and walk
> - * up the cpuset hierarchy until we find a suitable mask.
> + * task_cpu_possible_mask(). If the resulting mask is empty, we warn
> + * and walk up the cpuset hierarchy until we find a suitable mask.
> */
> void force_compatible_cpus_allowed_ptr(struct task_struct *p)
> {
> @@ -3125,11 +3130,15 @@ __sched_setaffinity(struct task_struct *p, struct affinity_context *ctx);
> void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
> {
> struct affinity_context ac = {
> - .new_mask = task_user_cpus(p),
> - .flags = 0,
> + .new_mask = cpu_possible_mask;
s/;/,/
But even with those two things fixed, I'm seeing new failures in my
testing which I think are because restrict_cpus_allowed_ptr() is failing
unexpectedly when called by force_compatible_cpus_allowed_ptr().
For example, just running a 32-bit task on an asymmetric system results
in:
$ ./hello32
[ 1690.855341] Overriding affinity for process 580 (hello32) to CPUs 2-3
That then has knock-on effects such as losing track of the initial affinity
mask and not being able to restore it if the forcefully-affined 32-bit task
exec()s a 64-bit program.
Will
On 1/27/23 07:59, Will Deacon wrote:
> Hi Waiman,
>
> On Thu, Jan 26, 2023 at 08:55:27PM -0500, Waiman Long wrote:
>> The user_cpus_ptr field was originally added by commit b90ca8badbd1
>> ("sched: Introduce task_struct::user_cpus_ptr to track requested
>> affinity"). It was used only by arm64 arch due to possible asymmetric
>> CPU setup.
>>
>> Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested
>> cpumask"), task_struct::user_cpus_ptr is repurposed to store user
>> requested cpu affinity specified in the sched_setaffinity().
>>
>> This results in a slight performance regression on an arm64
>> system when booted with "allow_mismatched_32bit_el0"
>> on the command-line. The arch code will (amongst
>> other things) calls force_compatible_cpus_allowed_ptr() and
>> relax_compatible_cpus_allowed_ptr() when exec()'ing a 32-bit or a 64-bit
>> task respectively. Now a call to relax_compatible_cpus_allowed_ptr()
>> will always result in a __sched_setaffinity() call whether there is a
>> previous force_compatible_cpus_allowed_ptr() call or not.
>>
>> In order to fix this regression, a new scheduler flag
>> task_struct::cpus_allowed_restricted is now added to track if
>> force_compatible_cpus_allowed_ptr() has been called before or not. This
>> patch also updates the comments in force_compatible_cpus_allowed_ptr()
>> and relax_compatible_cpus_allowed_ptr() and handles their interaction
>> with sched_setaffinity().
>>
>> This patch also removes the task_user_cpus() helper. In the case of
>> relax_compatible_cpus_allowed_ptr(), cpu_possible_mask as user_cpu_ptr
>> masking will be performed within __sched_setaffinity() anyway.
>>
>> Fixes: 8f9ea86fdf99 ("sched: Always preserve the user requested cpumask")
>> Reported-by: Will Deacon <[email protected]>
>> Signed-off-by: Waiman Long <[email protected]>
>> ---
>> include/linux/sched.h | 3 +++
>> kernel/sched/core.c | 25 +++++++++++++++++--------
>> kernel/sched/sched.h | 8 +-------
>> 3 files changed, 21 insertions(+), 15 deletions(-)
> So this doesn't even build...
>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index bb1ee6d7bdde..d7bc809c109e 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -2999,6 +2999,10 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,
>> struct rq *rq;
>>
>> rq = task_rq_lock(p, &rf);
>> +
>> + if (ctx->flags & SCA_CLR_RESTRICT)
>> + p->cpus_allowed_restricted = 0;
>> +
>> /*
>> * Masking should be skipped if SCA_USER or any of the SCA_MIGRATE_*
>> * flags are set.
>> @@ -3025,8 +3029,8 @@ EXPORT_SYMBOL_GPL(set_cpus_allowed_ptr);
>> /*
>> * Change a given task's CPU affinity to the intersection of its current
>> * affinity mask and @subset_mask, writing the resulting mask to @new_mask.
>> - * If user_cpus_ptr is defined, use it as the basis for restricting CPU
>> - * affinity or use cpu_online_mask instead.
>> + * The cpus_allowed_restricted bit is set to indicate to a later
>> + * relax_compatible_cpus_allowed_ptr() call to relax the cpumask.
>> *
>> * If the resulting mask is empty, leave the affinity unchanged and return
>> * -EINVAL.
>> @@ -3044,6 +3048,7 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
>> int err;
>>
>> rq = task_rq_lock(p, &rf);
>> + p->cpus_allowed_restricted = 1;
>>
>> /*
>> * Forcefully restricting the affinity of a deadline task is
>> @@ -3055,7 +3060,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
>> goto err_unlock;
>> }
>>
>> - if (!cpumask_and(new_mask, task_user_cpus(p), subset_mask)) {
>> + if (p->user_cpu_ptr &&
>> + !cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) {
> s/user_cpu_ptr/user_cpus_ptr/
>
>> err = -EINVAL;
>> goto err_unlock;
>> }
>> @@ -3069,9 +3075,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p,
>>
>> /*
>> * Restrict the CPU affinity of task @p so that it is a subset of
>> - * task_cpu_possible_mask() and point @p->user_cpus_ptr to a copy of the
>> - * old affinity mask. If the resulting mask is empty, we warn and walk
>> - * up the cpuset hierarchy until we find a suitable mask.
>> + * task_cpu_possible_mask(). If the resulting mask is empty, we warn
>> + * and walk up the cpuset hierarchy until we find a suitable mask.
>> */
>> void force_compatible_cpus_allowed_ptr(struct task_struct *p)
>> {
>> @@ -3125,11 +3130,15 @@ __sched_setaffinity(struct task_struct *p, struct affinity_context *ctx);
>> void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
>> {
>> struct affinity_context ac = {
>> - .new_mask = task_user_cpus(p),
>> - .flags = 0,
>> + .new_mask = cpu_possible_mask;
> s/;/,/
>
> But even with those two things fixed, I'm seeing new failures in my
> testing which I think are because restrict_cpus_allowed_ptr() is failing
> unexpectedly when called by force_compatible_cpus_allowed_ptr().
>
> For example, just running a 32-bit task on an asymmetric system results
> in:
>
> $ ./hello32
> [ 1690.855341] Overriding affinity for process 580 (hello32) to CPUs 2-3
>
> That then has knock-on effects such as losing track of the initial affinity
> mask and not being able to restore it if the forcefully-affined 32-bit task
> exec()s a 64-bit program.
I thought I have fixed the build failure. Apparently it is still there.
I will fix it.
BTW, which arm64 cpus support "allow_mismatched_32bit_el0"? I am trying
to see if I can reproduce the issue, but I am not sure if I have any
access to the cpus that have this capability.
Cheers,
Longman
Hi Waiman,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/sched/core]
[also build test ERROR on tip/master tip/auto-latest linux/master linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Waiman-Long/sched-Store-restrict_cpus_allowed_ptr-call-state/20230128-155441
patch link: https://lore.kernel.org/r/20230127015527.466367-1-longman%40redhat.com
patch subject: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state
config: hexagon-randconfig-r041-20230124 (https://download.01.org/0day-ci/archive/20230128/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/25582b263f75c0348a967ce5da36c1f87fd5d175
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Waiman-Long/sched-Store-restrict_cpus_allowed_ptr-call-state/20230128-155441
git checkout 25582b263f75c0348a967ce5da36c1f87fd5d175
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/bpf/ kernel/sched/ net/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from kernel/sched/core.c:9:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from kernel/sched/core.c:9:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from kernel/sched/core.c:9:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
>> kernel/sched/core.c:3021:9: error: no member named 'user_cpu_ptr' in 'struct task_struct'; did you mean 'user_cpus_ptr'?
if (p->user_cpu_ptr &&
^~~~~~~~~~~~
user_cpus_ptr
include/linux/sched.h:830:15: note: 'user_cpus_ptr' declared here
cpumask_t *user_cpus_ptr;
^
kernel/sched/core.c:3022:32: error: no member named 'user_cpu_ptr' in 'struct task_struct'; did you mean 'user_cpus_ptr'?
!cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) {
^~~~~~~~~~~~
user_cpus_ptr
include/linux/sched.h:830:15: note: 'user_cpus_ptr' declared here
cpumask_t *user_cpus_ptr;
^
>> kernel/sched/core.c:3091:33: error: expected '}'
.new_mask = cpu_possible_mask;
^
kernel/sched/core.c:3090:31: note: to match this '{'
struct affinity_context ac = {
^
>> kernel/sched/core.c:3092:3: error: expected expression
.flags = SCA_CLR_RESTRICT,
^
>> kernel/sched/core.c:3097:2: error: expected identifier or '('
if (!data_race(p->cpus_allowed_restricted))
^
>> kernel/sched/core.c:3104:2: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
ret = __sched_setaffinity(p, &ac);
^
int
>> kernel/sched/core.c:3104:28: error: use of undeclared identifier 'p'
ret = __sched_setaffinity(p, &ac);
^
>> kernel/sched/core.c:3104:32: error: use of undeclared identifier 'ac'
ret = __sched_setaffinity(p, &ac);
^
kernel/sched/core.c:3105:2: error: expected identifier or '('
WARN_ON_ONCE(ret);
^
include/asm-generic/bug.h:180:33: note: expanded from macro 'WARN_ON_ONCE'
#define WARN_ON_ONCE(condition) WARN_ON(condition)
^
include/asm-generic/bug.h:166:29: note: expanded from macro 'WARN_ON'
#define WARN_ON(condition) ({ \
^
>> kernel/sched/core.c:3105:2: error: expected ')'
include/asm-generic/bug.h:180:33: note: expanded from macro 'WARN_ON_ONCE'
#define WARN_ON_ONCE(condition) WARN_ON(condition)
^
include/asm-generic/bug.h:166:29: note: expanded from macro 'WARN_ON'
#define WARN_ON(condition) ({ \
^
kernel/sched/core.c:3105:2: note: to match this '('
include/asm-generic/bug.h:180:33: note: expanded from macro 'WARN_ON_ONCE'
#define WARN_ON_ONCE(condition) WARN_ON(condition)
^
include/asm-generic/bug.h:166:28: note: expanded from macro 'WARN_ON'
#define WARN_ON(condition) ({ \
^
>> kernel/sched/core.c:3106:1: error: extraneous closing brace ('}')
}
^
6 warnings and 11 errors generated.
vim +3021 kernel/sched/core.c
2986
2987 /*
2988 * Change a given task's CPU affinity to the intersection of its current
2989 * affinity mask and @subset_mask, writing the resulting mask to @new_mask.
2990 * The cpus_allowed_restricted bit is set to indicate to a later
2991 * relax_compatible_cpus_allowed_ptr() call to relax the cpumask.
2992 *
2993 * If the resulting mask is empty, leave the affinity unchanged and return
2994 * -EINVAL.
2995 */
2996 static int restrict_cpus_allowed_ptr(struct task_struct *p,
2997 struct cpumask *new_mask,
2998 const struct cpumask *subset_mask)
2999 {
3000 struct affinity_context ac = {
3001 .new_mask = new_mask,
3002 .flags = 0,
3003 };
3004 struct rq_flags rf;
3005 struct rq *rq;
3006 int err;
3007
3008 rq = task_rq_lock(p, &rf);
3009 p->cpus_allowed_restricted = 1;
3010
3011 /*
3012 * Forcefully restricting the affinity of a deadline task is
3013 * likely to cause problems, so fail and noisily override the
3014 * mask entirely.
3015 */
3016 if (task_has_dl_policy(p) && dl_bandwidth_enabled()) {
3017 err = -EPERM;
3018 goto err_unlock;
3019 }
3020
> 3021 if (p->user_cpu_ptr &&
> 3022 !cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) {
3023 err = -EINVAL;
3024 goto err_unlock;
3025 }
3026
3027 return __set_cpus_allowed_ptr_locked(p, &ac, rq, &rf);
3028
3029 err_unlock:
3030 task_rq_unlock(rq, p, &rf);
3031 return err;
3032 }
3033
3034 /*
3035 * Restrict the CPU affinity of task @p so that it is a subset of
3036 * task_cpu_possible_mask(). If the resulting mask is empty, we warn
3037 * and walk up the cpuset hierarchy until we find a suitable mask.
3038 */
3039 void force_compatible_cpus_allowed_ptr(struct task_struct *p)
3040 {
3041 cpumask_var_t new_mask;
3042 const struct cpumask *override_mask = task_cpu_possible_mask(p);
3043
3044 alloc_cpumask_var(&new_mask, GFP_KERNEL);
3045
3046 /*
3047 * __migrate_task() can fail silently in the face of concurrent
3048 * offlining of the chosen destination CPU, so take the hotplug
3049 * lock to ensure that the migration succeeds.
3050 */
3051 cpus_read_lock();
3052 if (!cpumask_available(new_mask))
3053 goto out_set_mask;
3054
3055 if (!restrict_cpus_allowed_ptr(p, new_mask, override_mask))
3056 goto out_free_mask;
3057
3058 /*
3059 * We failed to find a valid subset of the affinity mask for the
3060 * task, so override it based on its cpuset hierarchy.
3061 */
3062 cpuset_cpus_allowed(p, new_mask);
3063 override_mask = new_mask;
3064
3065 out_set_mask:
3066 if (printk_ratelimit()) {
3067 printk_deferred("Overriding affinity for process %d (%s) to CPUs %*pbl\n",
3068 task_pid_nr(p), p->comm,
3069 cpumask_pr_args(override_mask));
3070 }
3071
3072 WARN_ON(set_cpus_allowed_ptr(p, override_mask));
3073 out_free_mask:
3074 cpus_read_unlock();
3075 free_cpumask_var(new_mask);
3076 }
3077
3078 static int
3079 __sched_setaffinity(struct task_struct *p, struct affinity_context *ctx);
3080
3081 /*
3082 * Restore the affinity of a task @p which was previously restricted by a
3083 * call to force_compatible_cpus_allowed_ptr().
3084 *
3085 * It is the caller's responsibility to serialise this with any calls to
3086 * force_compatible_cpus_allowed_ptr(@p).
3087 */
3088 void relax_compatible_cpus_allowed_ptr(struct task_struct *p)
3089 {
> 3090 struct affinity_context ac = {
> 3091 .new_mask = cpu_possible_mask;
> 3092 .flags = SCA_CLR_RESTRICT,
3093 };
3094 int ret;
3095
3096 /* Return if no previous force_compatible_cpus_allowed_ptr() call */
> 3097 if (!data_race(p->cpus_allowed_restricted))
3098 return;
3099
3100 /*
3101 * Try to restore the old affinity mask with __sched_setaffinity().
3102 * Cpuset masking will be done there too.
3103 */
> 3104 ret = __sched_setaffinity(p, &ac);
> 3105 WARN_ON_ONCE(ret);
> 3106 }
3107
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Thu, Jan 26, 2023 at 08:55:27PM -0500, Waiman Long wrote:
> The user_cpus_ptr field was originally added by commit b90ca8badbd1
> ("sched: Introduce task_struct::user_cpus_ptr to track requested
> affinity"). It was used only by arm64 arch due to possible asymmetric
> CPU setup.
>
> Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested
> cpumask"), task_struct::user_cpus_ptr is repurposed to store user
> requested cpu affinity specified in the sched_setaffinity().
>
> This results in a slight performance regression on an arm64
> system when booted with "allow_mismatched_32bit_el0"
Dude, how can you still call this a slight performance regression after
Will told you time and time again that's not the problem.
It clearly is a behavioural problem.
On 1/30/23 06:56, Peter Zijlstra wrote:
> On Thu, Jan 26, 2023 at 08:55:27PM -0500, Waiman Long wrote:
>> The user_cpus_ptr field was originally added by commit b90ca8badbd1
>> ("sched: Introduce task_struct::user_cpus_ptr to track requested
>> affinity"). It was used only by arm64 arch due to possible asymmetric
>> CPU setup.
>>
>> Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested
>> cpumask"), task_struct::user_cpus_ptr is repurposed to store user
>> requested cpu affinity specified in the sched_setaffinity().
>>
>> This results in a slight performance regression on an arm64
>> system when booted with "allow_mismatched_32bit_el0"
> Dude, how can you still call this a slight performance regression after
> Will told you time and time again that's not the problem.
>
> It clearly is a behavioural problem.
I am trying to figure out if this behavioral problem is a result of my
scheduler patch or just as a result of cgroup v1 current behavior as I
don't see how my patch will cause this behavioral change if it is not an
existing problem.
Cheers,
Longman