Received-SPF: pass (google.com: domain of linux-kernel+bounces-205785-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 7 Jun 2024 15:32:36 +0530
From: Saket Kumar Bhaskar <skb99@linux.ibm.com>
To: Xuewen Yan <xuewen.yan@unisoc.com>
Cc: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, longman@redhat.com,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com,
        ke.wang@unisoc.com, xuewen.yan94@gmail.com,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] sched: Clear user_cpus_ptr only when no intersection
 with the new mask
Message-ID: <ZmLavE/tO66VUP3D@linux.ibm.com>
References: <20240507072242.585-1-xuewen.yan@unisoc.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240507072242.585-1-xuewen.yan@unisoc.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0

On Tue, May 07, 2024 at 03:22:42PM +0800, Xuewen Yan wrote:
> The commit 851a723e45d1c("sched: Always clear user_cpus_ptr in do_set_cpu=
s_allowed()")
> would cause that online/offline cpu will produce different results
> for the !top-cpuset task.
> For example:
>=20
> If the task was running, then offline task's cpus, would lead to clear
> its user-mask.
>=20
> unisoc:/ # while true; do sleep 600; done&
> [1] 6786
> unisoc:/ # echo 6786 > /dev/cpuset/top-app/tasks
> unisoc:/ # cat /dev/cpuset/top-app/cpus
> 0-7
> unisoc:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   ff
> Cpus_allowed_list:      0-7
>=20
> unisoc:/ # taskset -p c0 6786
> pid 6786's current affinity mask: ff
> pid 6786's new affinity mask: c0
> unisoc:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   c0
> Cpus_allowed_list:      6-7
>=20
> After offline the cpu6 and cpu7, the user-mask would be cleared:
>=20
> unisoc:/ # echo 0 > /sys/devices/system/cpu/cpu7/online
> unisoc:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   40
> Cpus_allowed_list:      6
> ums9621_1h10:/ # echo 0 > /sys/devices/system/cpu/cpu6/online
> ums9621_1h10:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   3f
> Cpus_allowed_list:      0-5
>=20
> When online the cpu6/7, the user-mask can not bring back:
>=20
> unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu6/online
> unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu7/online
> unisoc:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   ff
> Cpus_allowed_list:      0-6
>=20
> However, if we offline the cpu when the task is sleeping, at this
> time, because would not call the fallback_cpu(), its user-mask will
> not be cleared.
>=20
> unisoc:/ # while true; do sleep 600; done&
> [1] 5990
> unisoc:/ # echo 5990 > /dev/cpuset/top-app/tasks
> unisoc:/ # cat /proc/5990/status | grep Cpus
> Cpus_allowed:   ff
> Cpus_allowed_list:      0-7
>=20
> unisoc:/ # taskset -p c0 5990
> pid 5990's current affinity mask: ff
> pid 5990's new affinity mask: c0
> unisoc:/ # cat /proc/5990/status | grep Cpus
> Cpus_allowed:   c0
> Cpus_allowed_list:      6-7
>=20
> unisoc:/ # echo 0 > /sys/devices/system/cpu/cpu6/online
> unisoc:/ # cat /proc/5990/status | grep Cpus
> Cpus_allowed:   80
> Cpus_allowed_list:      7
> unisoc:/ # echo 0 > /sys/devices/system/cpu/cpu7/online
> unisoc:/ # cat /proc/5990/status | grep Cpus
> Cpus_allowed:   3f
> Cpus_allowed_list:      0-5
>=20
> After 10 minutes, it was waked up, it can also keep its user-mask:
> ums9621_1h10:/ # cat /proc/5990/status | grep Cpus
> Cpus_allowed:   3f
> Cpus_allowed_list:      0-5
>=20
> And when online the cpu6/7,the user-mask could bring back.
> unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu6/online
> unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu7/online
> unisoc:/ # cat /proc/6786/status | grep Cpus
> Cpus_allowed:   c0
> Cpus_allowed_list:      6-7
>=20
> Indeed, there is no need to clear the user_cpus_ptr if there is an
> intersection between user_cpus_ptr and new_mask.
> So add the judgement of whether there is an intersection between them.
> Clear user_cpus_ptr only when no intersection with the new mask.
> In this way, the above problems can also be solved.
>=20
> Suggested-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---
> previous discussion:
>  https://lore.kernel.org/all/e402d623-1875-47a2-9db3-8299a54502ef@redhat.=
com/
> ---
>  kernel/sched/core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>=20
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 7019a40457a6..bbb8e88949f4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2796,21 +2796,24 @@ __do_set_cpus_allowed(struct task_struct *p, stru=
ct affinity_context *ctx)
>  }
>=20=20
>  /*
> - * Used for kthread_bind() and select_fallback_rq(), in both cases the u=
ser
> - * affinity (if any) should be destroyed too.
> + * Used for kthread_bind() and select_fallback_rq().
> + * Destroy user affinity if no intersection with the new_mask.
>   */
>  void do_set_cpus_allowed(struct task_struct *p, const struct cpumask *ne=
w_mask)
>  {
>  	struct affinity_context ac =3D {
>  		.new_mask  =3D new_mask,
>  		.user_mask =3D NULL,
> -		.flags     =3D SCA_USER,	/* clear the user requested mask */
> +		.flags     =3D 0,
>  	};
>  	union cpumask_rcuhead {
>  		cpumask_t cpumask;
>  		struct rcu_head rcu;
>  	};
>=20=20
> +	if (p->user_cpus_ptr && !cpumask_intersects(p->user_cpus_ptr, new_mask))
> +		ac.flags =3D SCA_USER;	/* clear the user requested mask */
> +
>  	__do_set_cpus_allowed(p, &ac);
>=20=20
>  	/*
> --=20
> 2.25.1
>=20

Hi Xuewen,

I have a query here:

1. From the example where task is running, " while true; do sleep 600; done=
& "
   is being used, which is a sleeping task. How running task is emulated he=
re?

2. Tried without patch (6.9.0-rc4) on a 64 CPUs system. Results are:
=20=20=20
   For a task that is running:

   # stress-ng -l 100 --cpu 1
   stress-ng: info:  [2307] defaulting to a 86400 second (1 day, 0.00 secs)=
 run per stressor
   stress-ng: info:  [2307] dispatching hogs: 1 cpu
=20=20=20
   #taskset -p c0 2308
   pid 2308's current affinity mask: ffffffffffffffff
   pid 2308's new affinity mask: c0
=20=20=20
   # cat /proc/2308/status |grep Cpus
   Cpus_allowed:	00000000,000000c0
   Cpus_allowed_list:	6-7
=20=20=20
   #chcpu -d 6,7
   CPU 6 disabled
   CPU 7 disabled
=20=20=20
   After disabling CPUs 6 and 7:

   # cat /proc/2308/status |grep Cpus
   Cpus_allowed:	ffffffff,ffffffff
   Cpus_allowed_list:	0-63

   After enabling CPUs 6 and 7:

   ## chcpu -e 6,7
   CPU 6 enabled
   CPU 7 enabled
=20=20=20
   # cat /proc/2308/status |grep Cpus
   Cpus_allowed:	ffffffff,ffffffff
   Cpus_allowed_list:	0-63

   From the above output, after disabling CPUs 6 and 7, all the CPUs in the
   system are displayed rather than showing only remaining online CPUs(as=20
   shown in above example).

   For a task that is sleeping:

   # while true; do sleep 60; done&
   [1] 2541
=20=20=20
   # taskset -p c0 2541
   pid 2541's current affinity mask: ffffffffffffffff
   pid 2541's new affinity mask: c0
=20=20=20
   # cat /proc/2541/status |grep Cpus
   Cpus_allowed:	00000000,000000c0
   Cpus_allowed_list:	6-7
=20
   After disabling CPUs 6 and 7:
=20=20
   # chcpu -d 6,7
   CPU 6 disabled
   CPU 7 disabled

   # cat /proc/2541/status |grep Cpus
   Cpus_allowed:	00000000,000000c0
   Cpus_allowed_list:	6-7
=20=20=20
   After 1 minute:

   # cat /proc/2541/status |grep Cpus
   Cpus_allowed:	ffffffff,ffffffff
   Cpus_allowed_list:	0-63
=20=20=20
   # chcpu -e 6,7
   CPU 6 enabled
   CPU 7 enabled
=20=20=20
   # cat /proc/2541/status |grep Cpus
   Cpus_allowed:	ffffffff,ffffffff
   Cpus_allowed_list:	0-63

   From the above output, after disabling CPUs 6 and 7, it waked up after=20
   1 minute and Cpus_allowed_list got changed to 0-63 (which is contrary=20
   to the above example).

   So, there is some deviation in behaviour seen without the patch,=20
   than reported or am I missing something?

Thanks and regards,
Saket Kumar Bhaskar