Received-SPF: pass (google.com: domain of linux-kernel+bounces-169467-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240429084633.9800-1-xuewen.yan@unisoc.com> <20240429121000.GA40213@noisy.programming.kicks-ass.net>
In-Reply-To: <20240429121000.GA40213@noisy.programming.kicks-ass.net>
From: Xuewen Yan <xuewen.yan94@gmail.com>
Date: Mon, 6 May 2024 16:04:22 +0800
Message-ID: <CAB8ipk831xtAW2+sm-evm-oOsFspL=xSp6hFYYq1uKmWA+porQ@mail.gmail.com>
Subject: Re: [PATCH] sched/proc: Print user_cpus_ptr for task status
To: Peter Zijlstra <peterz@infradead.org>
Cc: Xuewen Yan <xuewen.yan@unisoc.com>, akpm@linux-foundation.org, oleg@redhat.com, 
	longman@redhat.com, dylanbhatch@google.com, rick.p.edgecombe@intel.com, 
	ke.wang@unisoc.com, linux-kernel@vger.kernel.org, 
	linux-fsdevel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Peter

On Mon, Apr 29, 2024 at 8:10=E2=80=AFPM Peter Zijlstra <peterz@infradead.or=
g> wrote:
>
> On Mon, Apr 29, 2024 at 04:46:33PM +0800, Xuewen Yan wrote:
> > The commit 851a723e45d1c("sched: Always clear user_cpus_ptr in do_set_c=
pus_allowed()")
> > would clear the user_cpus_ptr when call the do_set_cpus_allowed.
> >
> > In order to determine whether the user_cpus_ptr is taking effect,
> > it is better to print the task's user_cpus_ptr.
>
> This is an ABI change and would mandate we forever more have this
> distinction. I don't think your changes justifies things sufficiently
> for this.

I added this mainly because online/offline cpu will produce different
results for the !top-cpuset task.

For example:

If the task was running, then offline task's cpus, would lead to clear
its user-mask.

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
Cpus_user_allowed:        (null)
Cpus_user_allowed_list:   (null)

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
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-7

After offline the cpu6 and cpu7, the user-mask would be cleared:

unisoc:/ # echo 0 > /sys/devices/system/cpu/cpu7/online
unisoc:/ # cat /proc/6786/status | grep Cpus
Cpus_allowed:   40
Cpus_allowed_list:      6
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-7
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
Cpus_user_allowed:        (null)
Cpus_user_allowed_list:   (null)

When online the cpu6/7, the user-mask can not bring back:

unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu6/online
unisoc:/ # cat /proc/6786/status | grep Cpus
Cpus_allowed:   7f
Cpus_allowed_list:      0-6
Cpus_user_allowed:        (null)
Cpus_user_allowed_list:   (null)
unisoc:/ # echo 1 > /sys/devices/system/cpu/cpu7/online
unisoc:/ # cat /proc/6786/status | grep Cpus
Cpus_allowed:   ff
Cpus_allowed_list:      0-7
Cpus_user_allowed:        (null)
Cpus_user_allowed_list:   (null)

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.

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
Cpus_user_allowed:        (null)
Cpus_user_allowed_list:   (null)

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
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-7

unisoc:/ # echo 0 > /sys/devices/system/cpu/cpu6/online
unisoc:/ # cat /proc/5990/status | grep Cpus
Cpus_allowed:   80
Cpus_allowed_list:      7
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-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
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-7


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
Cpus_user_allowed:      c0
Cpus_user_allowed_list: 6-7

In order to solve the above problem, I modified the following patch.
At this time, for !top-cpuset, regardless of whether the task is in
the running state when offline cpu, its cpu-mask can be maintained.
However, this patch may not be perfect yet, so I send the "Print
user_cpus_ptr for task status" patch first to debug more conveniently.

--->

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 68cfa656b9b1..00879b6de8d4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1870,7 +1870,7 @@ extern void dl_bw_free(int cpu, u64 dl_bw);
 #ifdef CONFIG_SMP

 /* do_set_cpus_allowed() - consider using set_cpus_allowed_ptr() instead *=
/
-extern void do_set_cpus_allowed(struct task_struct *p, const struct
cpumask *new_mask);
+extern void do_set_cpus_allowed(struct task_struct *p, const struct
cpumask *new_mask, bool keep_user);

 /**
  * set_cpus_allowed_ptr - set CPU affinity mask of a task
@@ -1886,7 +1886,7 @@ extern int dl_task_check_affinity(struct
task_struct *p, const struct cpumask *m
 extern void force_compatible_cpus_allowed_ptr(struct task_struct *p);
 extern void relax_compatible_cpus_allowed_ptr(struct task_struct *p);
 #else
-static inline void do_set_cpus_allowed(struct task_struct *p, const
struct cpumask *new_mask)
+static inline void do_set_cpus_allowed(struct task_struct *p, const
struct cpumask *new_mask, bool keep_user)
 {
 }
 static inline int set_cpus_allowed_ptr(struct task_struct *p, const
struct cpumask *new_mask)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7ee9994aee40..0c448f8a3829 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4005,9 +4005,14 @@ bool cpuset_cpus_allowed_fallback(struct
task_struct *tsk)

        rcu_read_lock();
        cs_mask =3D task_cs(tsk)->cpus_allowed;
-       if (is_in_v2_mode() && cpumask_subset(cs_mask, possible_mask)) {
-               do_set_cpus_allowed(tsk, cs_mask);
-               changed =3D true;
+       if (cpumask_subset(cs_mask, possible_mask)) {
+               if (is_in_v2_mode()) {
+                       do_set_cpus_allowed(tsk, cs_mask, false);
+                       changed =3D true;
+               } else if (task_cs(tsk) !=3D &top_cpuset) {
+                       do_set_cpus_allowed(tsk, cs_mask, true);
+                       changed =3D true;
+               }
        }
        rcu_read_unlock();

diff --git a/kernel/kthread.c b/kernel/kthread.c
index 7a7aa5f93c0c..7ede27630088 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -527,7 +527,7 @@ static void __kthread_bind_mask(struct task_struct
*p, const struct cpumask *mas

        /* It's safe because the task is inactive. */
        raw_spin_lock_irqsave(&p->pi_lock, flags);
-       do_set_cpus_allowed(p, mask);
+       do_set_cpus_allowed(p, mask, false);
        p->flags |=3D PF_NO_SETAFFINITY;
        raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 }
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 33cfd522fc7c..623f89e65e6c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2855,18 +2855,21 @@ __do_set_cpus_allowed(struct task_struct *p,
struct affinity_context *ctx)
  * Used for kthread_bind() and select_fallback_rq(), in both cases the use=
r
  * affinity (if any) should be destroyed too.
  */
-void do_set_cpus_allowed(struct task_struct *p, const struct cpumask *new_=
mask)
+void do_set_cpus_allowed(struct task_struct *p, const struct cpumask
*new_mask, bool keep_user)
 {
        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, /* clear the user requested mask */
        };
        union cpumask_rcuhead {
                cpumask_t cpumask;
                struct rcu_head rcu;
        };

+       if (!keep_user)
+               ac.flags =3D SCA_USER;
+
        __do_set_cpus_allowed(p, &ac);

        /*
@@ -2874,7 +2877,8 @@ void do_set_cpus_allowed(struct task_struct *p,
const struct cpumask *new_mask)
         * to use kfree() here (when PREEMPT_RT=3Dy), therefore punt to usi=
ng
         * kfree_rcu().
         */
-       kfree_rcu((union cpumask_rcuhead *)ac.user_mask, rcu);
+       if (!keep_user)
+               kfree_rcu((union cpumask_rcuhead *)ac.user_mask, rcu);
 }

 static cpumask_t *alloc_user_cpus_ptr(int node)
@@ -3664,7 +3668,7 @@ int select_fallback_rq(int cpu, struct task_struct *p=
)
                         *
                         * More yuck to audit.
                         */
-                       do_set_cpus_allowed(p, task_cpu_possible_mask(p));
+                       do_set_cpus_allowed(p,
task_cpu_possible_mask(p), false);
                        state =3D fail;
                        break;
                case fail:


---
xuewen