[PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Waiman Long]

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 <will@kernel.org>
Signed-off-by: Waiman Long <longman@redhat.com>
---
 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

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Linux kernel regression tracking (Thorsten Leemhuis)]

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 <will@kernel.org>

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.

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Will Deacon]

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 <will@kernel.org>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  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

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Waiman Long]

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 <will@kernel.org>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   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

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[kernel test robot]

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/202301282334.C5NLDEZd-lkp@intel.com/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 <lkp@intel.com>

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

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Peter Zijlstra]

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.

Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state

[Waiman Long]

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