lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA1PR11MB61717873D4697DBB04526F6489D89@IA1PR11MB6171.namprd11.prod.outlook.com>
Date:   Wed, 8 Feb 2023 13:12:41 +0000
From:   "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
To:     "Zhang, Qiang1" <qiang1.zhang@...el.com>,
        "paulmck@...nel.org" <paulmck@...nel.org>,
        "frederic@...nel.org" <frederic@...nel.org>,
        "joel@...lfernandes.org" <joel@...lfernandes.org>
CC:     "rcu@...r.kernel.org" <rcu@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] rcu: Fix bind rcu related kthreads to housekeeping CPUs

Did the testing and checked the CPU affinities of RCU kthreads by running "ps -eo psr,comm| grep rcu" [1]:

Without Zqiang's patch:
    ...
    0 rcu_preempt
    0 rcuog/0
    0 rcuop/0
    0 rcuop/1
    0 rcuog/2
    0 rcuop/2
    2 rcuop/3

With Zqiang's patch:
    ...
    3 rcu_preempt   // on housekeeping CPU 3
    2 rcuog/0            // on housekeeping CPU 2
    3 rcuop/0            // on housekeeping CPU 3
    3 rcuop/1            // on housekeeping CPU 3
    0 rcuog/2            // on non-housekeeping CPU 0.  [2]
    0 rcuop/2            // on non-housekeeping CPU 0.  [2]
    2 rcuop/3

[1] The 1st column of the output is for CPU IDs and the 2nd column is for thread names. 
[2] Added debug messages into the two threads, and found that the two threads didn't run after all CPUs were online.
     So if they run again after all CPUs are online, they will also be moved to housekeeping CPUs by Zqiang's patch. 

> From: Zqiang <qiang1.zhang@...el.com>
> Sent: Wednesday, February 8, 2023 11:34 AM
> To: paulmck@...nel.org; frederic@...nel.org; joel@...lfernandes.org
> Cc: rcu@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: [PATCH] rcu: Fix bind rcu related kthreads to housekeeping CPUs
> 
> For kernels built with CONFIG_NO_HZ_FULL=y and CONFIG_RCU_NOCB_CPU=y,
> run the following tests:
> 
> runqemu kvm slirp nographic qemuparams="-m 1024 -smp 4"
> bootparams="console=ttyS0 isolcpus=0,1 nohz_full=0,1 rcu_nocbs=0,1"
> 
> root@...ux86-64:~# ps -ef | grep "rcu_preempt" | grep -v grep | awk '{print
> $2}'
> 15
> root@...ux86-64:~# ps -ef | grep "rcuop/0" | grep -v grep | awk '{print $2}'
> 17
> root@...ux86-64:~# taskset -p 15
> pid 15's current affinity mask: 1
> root@...ux86-64:~# taskset -p 17
> pid 17's current affinity mask: 1
> 
> The affinity of these rcu related kthreads is not set to housekeeping cpumask,
> even if called rcu_bind_gp_kthread() when the rcu related kthread starts.
> 
> set_cpus_allowed_ptr()
>  ->__set_cpus_allowed_ptr()
>    ->__set_cpus_allowed_ptr_locked
>      {
> 		bool kthread = p->flags & PF_KTHREAD;
> 		....
> 		if (kthread || is_migration_disabled(p))
> 			cpu_valid_mask = cpu_online_mask;
> 		....
> 		dest_cpu = cpumask_any_and_distribute(cpu_valid_mask, ctx-
> >new_mask);
> 		if (dest_cpu >= nr_cpu_ids) {
> 			ret = -EINVAL;
> 			goto out;
> 		}
> 		....
>      }
> 
> Due to these rcu related kthreads be created before bringup other CPUS, so
> when they running and set hosekeeping cpus affinity, found that only CPU0 is
> online at this time and CPU0 is set to no_hz_full CPU, the ctx->new_mask not
> contain CPU0 and this will cause dest_cpu in the above code snippet to be an
> illegal value and return directly, ultimately, these rcu related kthreads failed to
> bind housekeeping CPUS.

s/so//
s/hosekeeping/housekeeping/
...

> This commit therefore rebind these rcu related kthreads to housekeeping CPUs

s/rebind/rebinds/

Could you tweak all the commit messages to fix the typos and the grammar errors? 😊
Other than that, 

    Tested-by: Qiuxu Zhuo <qiuxu.zhuo@...el.com>

> after the kernel boot sequence ends, at this point all CPUs are online.
> 
> Signed-off-by: Zqiang <qiang1.zhang@...el.com>
> ---
>  kernel/rcu/tasks.h       |  7 +++++--
>  kernel/rcu/tree.c        |  7 ++++++-
>  kernel/rcu/tree.h        |  1 -
>  kernel/rcu/tree_nocb.h   | 13 ++++++++++++-
>  kernel/rcu/tree_plugin.h |  9 ---------
>  5 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index
> baf7ec178155..8b3530cca291 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -544,9 +544,8 @@ static void rcu_tasks_one_gp(struct rcu_tasks *rtp, bool
> midboot)  static int __noreturn rcu_tasks_kthread(void *arg)  {
>  	struct rcu_tasks *rtp = arg;
> +	bool rcu_setaffinity_setup = false;
> 
> -	/* Run on housekeeping CPUs by default.  Sysadm can move if desired.
> */
> -	housekeeping_affine(current, HK_TYPE_RCU);
>  	WRITE_ONCE(rtp->kthread_ptr, current); // Let GPs start!
> 
>  	/*
> @@ -556,6 +555,10 @@ static int __noreturn rcu_tasks_kthread(void *arg)
>  	 * This loop is terminated by the system going down.  ;-)
>  	 */
>  	for (;;) {
> +		if (!rcu_setaffinity_setup && rcu_inkernel_boot_has_ended()) {
> +			set_cpus_allowed_ptr(current,
> housekeeping_cpumask(HK_TYPE_RCU));
> +			rcu_setaffinity_setup = true;
> +		}
>  		// Wait for one grace period and invoke any callbacks
>  		// that are ready.
>  		rcu_tasks_one_gp(rtp, false);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index
> ee27a03d7576..0ac47a773e13 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1781,8 +1781,13 @@ static noinline void rcu_gp_cleanup(void)
>   */
>  static int __noreturn rcu_gp_kthread(void *unused)  {
> -	rcu_bind_gp_kthread();
> +	bool rcu_setaffinity_setup = false;
> +
>  	for (;;) {
> +		if (!rcu_setaffinity_setup && rcu_inkernel_boot_has_ended()) {
> +			set_cpus_allowed_ptr(current,
> housekeeping_cpumask(HK_TYPE_RCU));
> +			rcu_setaffinity_setup = true;
> +		}
> 
>  		/* Handle grace-period start. */
>  		for (;;) {
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h index
> 192536916f9a..391e3fae4ff5 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -495,7 +495,6 @@ do {
> 		\
>  #define rcu_nocb_lock_irqsave(rdp, flags) local_irq_save(flags)  #endif /* #else
> #ifdef CONFIG_RCU_NOCB_CPU */
> 
> -static void rcu_bind_gp_kthread(void);
>  static bool rcu_nohz_full_cpu(void);
> 
>  /* Forward declarations for tree_stall.h */ diff --git a/kernel/rcu/tree_nocb.h
> b/kernel/rcu/tree_nocb.h index f2280616f9d5..254d0f631d57 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -894,8 +894,14 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
> static int rcu_nocb_gp_kthread(void *arg)  {
>  	struct rcu_data *rdp = arg;
> +	bool rcu_setaffinity_setup = false;
> 
>  	for (;;) {
> +		if (!rcu_setaffinity_setup && rcu_inkernel_boot_has_ended()) {
> +			set_cpus_allowed_ptr(current,
> housekeeping_cpumask(HK_TYPE_RCU));
> +			rcu_setaffinity_setup = true;
> +		}
> +
>  		WRITE_ONCE(rdp->nocb_gp_loops, rdp->nocb_gp_loops + 1);
>  		nocb_gp_wait(rdp);
>  		cond_resched_tasks_rcu_qs();
> @@ -1002,10 +1008,15 @@ static void nocb_cb_wait(struct rcu_data *rdp)
> static int rcu_nocb_cb_kthread(void *arg)  {
>  	struct rcu_data *rdp = arg;
> -
> +	bool rcu_setaffinity_setup = false;
>  	// Each pass through this loop does one callback batch, and,
>  	// if there are no more ready callbacks, waits for them.
>  	for (;;) {
> +		if (!rcu_setaffinity_setup && rcu_inkernel_boot_has_ended()) {
> +			set_cpus_allowed_ptr(current,
> housekeeping_cpumask(HK_TYPE_RCU));
> +			rcu_setaffinity_setup = true;
> +		}
> +
>  		nocb_cb_wait(rdp);
>  		cond_resched_tasks_rcu_qs();
>  	}
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index
> 7b0fe741a088..fdde71ebb83e 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1294,12 +1294,3 @@ static bool rcu_nohz_full_cpu(void)
>  	return false;
>  }
> 
> -/*
> - * Bind the RCU grace-period kthreads to the housekeeping CPU.
> - */
> -static void rcu_bind_gp_kthread(void)
> -{
> -	if (!tick_nohz_full_enabled())
> -		return;
> -	housekeeping_affine(current, HK_TYPE_RCU);
> -}
> --
> 2.25.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ