Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1401622 > unrolled thread

[PATCH] rcu: tree: correctly handle sparse possible CPUs

Started byMark Rutland <mark.rutland@arm.com>
First post2016-05-16 18:50 +0200
Last post2016-05-18 20:50 +0200
Articles 12 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] rcu: tree: correctly handle sparse possible CPUs Mark Rutland <mark.rutland@arm.com> - 2016-05-16 18:50 +0200
    Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-16 21:30 +0200
      [PATCHv2] rcu: tree: correctly handle sparse possible CPUs Mark Rutland <mark.rutland@arm.com> - 2016-05-17 12:30 +0200
        Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-17 21:10 +0200
          Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs Mark Rutland <mark.rutland@arm.com> - 2016-05-18 08:10 +0200
          Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs Arnd Bergmann <arnd@arndb.de> - 2016-05-18 14:10 +0200
            Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs Mark Rutland <mark.rutland@arm.com> - 2016-05-18 20:20 +0200
              Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-18 20:50 +0200
    Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs Andrey Ryabinin <ryabinin.a.a@gmail.com> - 2016-05-18 17:20 +0200
      Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-18 20:10 +0200
        Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs Mark Rutland <mark.rutland@arm.com> - 2016-05-18 20:40 +0200
          Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-05-18 20:50 +0200

#1401622 — [PATCH] rcu: tree: correctly handle sparse possible CPUs

FromMark Rutland <mark.rutland@arm.com>
Date2016-05-16 18:50 +0200
Subject[PATCH] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rztSq-10v-15@gated-at.bofh.it>
In many cases in the RCU tree code, we iterate over the set of CPUS for
a leaf node described by rcu_node::grplo and rcu_node::grphi, checking
per-cpu data for each CPU in this range. However, if the set of possible
CPUs is sparse, some CPUs described in this range are not possible, and
thus no per-cpu region will have been allocated (or initialised) for
them by the generic percpu code.

Erroneous accesses to a per-cpu area for these !possible CPUs may fault
or may hit other data depending on the addressed generated when the
erroneous per cpu offset is applied. In practice, both cases have been
observed on arm64 hardware (the former being silent, but detectable with
additional patches).

To avoid issues resulting from this, we must iterate over the set of
*possible* cpus for a given leaf node. This patch adds new helpers to
enable this (also unifying and simplifying some related bitmask
manipulation logic), and moves the RCU tree code over to them.

Without this patch, running reboot at a shell can result in an oops
like:

[ 3369.075979] Unable to handle kernel paging request at virtual address ffffff8008b21b4c
[ 3369.083881] pgd = ffffffc3ecdda000
[ 3369.087270] [ffffff8008b21b4c] *pgd=00000083eca48003, *pud=00000083eca48003, *pmd=0000000000000000
[ 3369.096222] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[ 3369.101781] Modules linked in:
[ 3369.104825] CPU: 2 PID: 1817 Comm: NetworkManager Tainted: G        W       4.6.0+ #3
[ 3369.121239] task: ffffffc0fa13e000 ti: ffffffc3eb940000 task.ti: ffffffc3eb940000
[ 3369.128708] PC is at sync_rcu_exp_select_cpus+0x188/0x510
[ 3369.134094] LR is at sync_rcu_exp_select_cpus+0x104/0x510
[ 3369.139479] pc : [<ffffff80081109a8>] lr : [<ffffff8008110924>] pstate: 200001c5
[ 3369.146860] sp : ffffffc3eb9435a0
[ 3369.150162] x29: ffffffc3eb9435a0 x28: ffffff8008be4f88
[ 3369.155465] x27: ffffff8008b66c80 x26: ffffffc3eceb2600
[ 3369.160767] x25: 0000000000000001 x24: ffffff8008be4f88
[ 3369.166070] x23: ffffff8008b51c3c x22: ffffff8008b66c80
[ 3369.171371] x21: 0000000000000001 x20: ffffff8008b21b40
[ 3369.176673] x19: ffffff8008b66c80 x18: 0000000000000000
[ 3369.181975] x17: 0000007fa951a010 x16: ffffff80086a30f0
[ 3369.187278] x15: 0000007fa9505590 x14: 0000000000000000
[ 3369.192580] x13: ffffff8008b51000 x12: ffffffc3eb940000
[ 3369.197882] x11: 0000000000000006 x10: ffffff8008b51b78
[ 3369.203184] x9 : 0000000000000001 x8 : ffffff8008be4000
[ 3369.208486] x7 : ffffff8008b21b40 x6 : 0000000000001003
[ 3369.213788] x5 : 0000000000000000 x4 : ffffff8008b27280
[ 3369.219090] x3 : ffffff8008b21b4c x2 : 0000000000000001
[ 3369.224406] x1 : 0000000000000001 x0 : 0000000000000140
...
[ 3369.972257] [<ffffff80081109a8>] sync_rcu_exp_select_cpus+0x188/0x510
[ 3369.978685] [<ffffff80081128b4>] synchronize_rcu_expedited+0x64/0xa8
[ 3369.985026] [<ffffff80086b987c>] synchronize_net+0x24/0x30
[ 3369.990499] [<ffffff80086ddb54>] dev_deactivate_many+0x28c/0x298
[ 3369.996493] [<ffffff80086b6bb8>] __dev_close_many+0x60/0xd0
[ 3370.002052] [<ffffff80086b6d48>] __dev_close+0x28/0x40
[ 3370.007178] [<ffffff80086bf62c>] __dev_change_flags+0x8c/0x158
[ 3370.012999] [<ffffff80086bf718>] dev_change_flags+0x20/0x60
[ 3370.018558] [<ffffff80086cf7f0>] do_setlink+0x288/0x918
[ 3370.023771] [<ffffff80086d0798>] rtnl_newlink+0x398/0x6a8
[ 3370.029158] [<ffffff80086cee84>] rtnetlink_rcv_msg+0xe4/0x220
[ 3370.034891] [<ffffff80086e274c>] netlink_rcv_skb+0xc4/0xf8
[ 3370.040364] [<ffffff80086ced8c>] rtnetlink_rcv+0x2c/0x40
[ 3370.045663] [<ffffff80086e1fe8>] netlink_unicast+0x160/0x238
[ 3370.051309] [<ffffff80086e24b8>] netlink_sendmsg+0x2f0/0x358
[ 3370.056956] [<ffffff80086a0070>] sock_sendmsg+0x18/0x30
[ 3370.062168] [<ffffff80086a21cc>] ___sys_sendmsg+0x26c/0x280
[ 3370.067728] [<ffffff80086a30ac>] __sys_sendmsg+0x44/0x88
[ 3370.073027] [<ffffff80086a3100>] SyS_sendmsg+0x10/0x20
[ 3370.078153] [<ffffff8008085e70>] el0_svc_naked+0x24/0x28

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Dennis Chen <dennis.chen@arm.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/rcu/tree.c | 30 +++++++++++++-----------------
 kernel/rcu/tree.h | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 9a535a8..2923df3 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1235,15 +1235,16 @@ static void rcu_check_gp_kthread_starvation(struct rcu_state *rsp)
 static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
 {
 	int cpu;
+	unsigned long bit;
 	unsigned long flags;
 	struct rcu_node *rnp;
 
 	rcu_for_each_leaf_node(rsp, rnp) {
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 		if (rnp->qsmask != 0) {
-			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
-				if (rnp->qsmask & (1UL << cpu))
-					dump_cpu_task(rnp->grplo + cpu);
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
+				if (rnp->qsmask & bit)
+					dump_cpu_task(cpu);
 		}
 		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 	}
@@ -1252,6 +1253,7 @@ static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
 static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
 {
 	int cpu;
+	unsigned long bit;
 	long delta;
 	unsigned long flags;
 	unsigned long gpa;
@@ -1284,10 +1286,9 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 		ndetected += rcu_print_task_stall(rnp);
 		if (rnp->qsmask != 0) {
-			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
-				if (rnp->qsmask & (1UL << cpu)) {
-					print_cpu_stall_info(rsp,
-							     rnp->grplo + cpu);
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
+				if (rnp->qsmask & bit) {
+					print_cpu_stall_info(rsp, cpu);
 					ndetected++;
 				}
 		}
@@ -2823,9 +2824,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
 				continue;
 			}
 		}
-		cpu = rnp->grplo;
-		bit = 1;
-		for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
+		for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) {
 			if ((rnp->qsmask & bit) != 0) {
 				if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
 					mask |= bit;
@@ -3690,7 +3689,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 
 		/* Each pass checks a CPU for identity, offline, and idle. */
 		mask_ofl_test = 0;
-		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++) {
+		for_each_leaf_node_possible_cpu(rnp, cpu) {
 			struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
 			struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
 
@@ -3710,8 +3709,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 
 		/* IPI the remaining CPUs for expedited quiescent state. */
-		mask = 1;
-		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+		for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 			if (!(mask_ofl_ipi & mask))
 				continue;
 retry_ipi:
@@ -3774,8 +3772,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 		ndetected = 0;
 		rcu_for_each_leaf_node(rsp, rnp) {
 			ndetected = rcu_print_task_exp_stall(rnp);
-			mask = 1;
-			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 				struct rcu_data *rdp;
 
 				if (!(rnp->expmask & mask))
@@ -3807,8 +3804,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 			pr_cont("\n");
 		}
 		rcu_for_each_leaf_node(rsp, rnp) {
-			mask = 1;
-			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 				if (!(rnp->expmask & mask))
 					continue;
 				dump_cpu_task(cpu);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index df668c0..d823b8e 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -283,6 +283,24 @@ struct rcu_node {
 	     (rnp) < &(rsp)->node[rcu_num_nodes]; (rnp)++)
 
 /*
+ * Iterate over all possible CPUs in a leaf RCU node.
+ */
+#define for_each_leaf_node_possible_cpu(rnp, cpu) \
+	for ((cpu) = rnp->grplo; \
+	     cpu <= rnp->grphi; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask))
+
+/*
+ * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
+ * bit for comparison against rcu_node bitmasks.
+ */
+#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
+	for ((cpu) = rnp->grplo, (bit) = 1; \
+	     cpu <= rnp->grphi; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask), \
+		   (bit) = 1UL << (cpu - rnp->grplo))
+
+/*
  * Union to allow "aggregate OR" operation on the need for a quiescent
  * state by the normal and expedited grace periods.
  */
-- 
1.9.1

[toc] | [next] | [standalone]

#1401720

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-05-16 21:30 +0200
Message-ID<rzwng-2Fv-9@gated-at.bofh.it>
In reply to#1401622
On Mon, May 16, 2016 at 05:48:26PM +0100, Mark Rutland wrote:
> In many cases in the RCU tree code, we iterate over the set of CPUS for
> a leaf node described by rcu_node::grplo and rcu_node::grphi, checking
> per-cpu data for each CPU in this range. However, if the set of possible
> CPUs is sparse, some CPUs described in this range are not possible, and
> thus no per-cpu region will have been allocated (or initialised) for
> them by the generic percpu code.
> 
> Erroneous accesses to a per-cpu area for these !possible CPUs may fault
> or may hit other data depending on the addressed generated when the
> erroneous per cpu offset is applied. In practice, both cases have been
> observed on arm64 hardware (the former being silent, but detectable with
> additional patches).
> 
> To avoid issues resulting from this, we must iterate over the set of
> *possible* cpus for a given leaf node. This patch adds new helpers to
> enable this (also unifying and simplifying some related bitmask
> manipulation logic), and moves the RCU tree code over to them.
> 
> Without this patch, running reboot at a shell can result in an oops
> like:
> 
> [ 3369.075979] Unable to handle kernel paging request at virtual address ffffff8008b21b4c
> [ 3369.083881] pgd = ffffffc3ecdda000
> [ 3369.087270] [ffffff8008b21b4c] *pgd=00000083eca48003, *pud=00000083eca48003, *pmd=0000000000000000
> [ 3369.096222] Internal error: Oops: 96000007 [#1] PREEMPT SMP
> [ 3369.101781] Modules linked in:
> [ 3369.104825] CPU: 2 PID: 1817 Comm: NetworkManager Tainted: G        W       4.6.0+ #3
> [ 3369.121239] task: ffffffc0fa13e000 ti: ffffffc3eb940000 task.ti: ffffffc3eb940000
> [ 3369.128708] PC is at sync_rcu_exp_select_cpus+0x188/0x510
> [ 3369.134094] LR is at sync_rcu_exp_select_cpus+0x104/0x510
> [ 3369.139479] pc : [<ffffff80081109a8>] lr : [<ffffff8008110924>] pstate: 200001c5
> [ 3369.146860] sp : ffffffc3eb9435a0
> [ 3369.150162] x29: ffffffc3eb9435a0 x28: ffffff8008be4f88
> [ 3369.155465] x27: ffffff8008b66c80 x26: ffffffc3eceb2600
> [ 3369.160767] x25: 0000000000000001 x24: ffffff8008be4f88
> [ 3369.166070] x23: ffffff8008b51c3c x22: ffffff8008b66c80
> [ 3369.171371] x21: 0000000000000001 x20: ffffff8008b21b40
> [ 3369.176673] x19: ffffff8008b66c80 x18: 0000000000000000
> [ 3369.181975] x17: 0000007fa951a010 x16: ffffff80086a30f0
> [ 3369.187278] x15: 0000007fa9505590 x14: 0000000000000000
> [ 3369.192580] x13: ffffff8008b51000 x12: ffffffc3eb940000
> [ 3369.197882] x11: 0000000000000006 x10: ffffff8008b51b78
> [ 3369.203184] x9 : 0000000000000001 x8 : ffffff8008be4000
> [ 3369.208486] x7 : ffffff8008b21b40 x6 : 0000000000001003
> [ 3369.213788] x5 : 0000000000000000 x4 : ffffff8008b27280
> [ 3369.219090] x3 : ffffff8008b21b4c x2 : 0000000000000001
> [ 3369.224406] x1 : 0000000000000001 x0 : 0000000000000140
> ...
> [ 3369.972257] [<ffffff80081109a8>] sync_rcu_exp_select_cpus+0x188/0x510
> [ 3369.978685] [<ffffff80081128b4>] synchronize_rcu_expedited+0x64/0xa8
> [ 3369.985026] [<ffffff80086b987c>] synchronize_net+0x24/0x30
> [ 3369.990499] [<ffffff80086ddb54>] dev_deactivate_many+0x28c/0x298
> [ 3369.996493] [<ffffff80086b6bb8>] __dev_close_many+0x60/0xd0
> [ 3370.002052] [<ffffff80086b6d48>] __dev_close+0x28/0x40
> [ 3370.007178] [<ffffff80086bf62c>] __dev_change_flags+0x8c/0x158
> [ 3370.012999] [<ffffff80086bf718>] dev_change_flags+0x20/0x60
> [ 3370.018558] [<ffffff80086cf7f0>] do_setlink+0x288/0x918
> [ 3370.023771] [<ffffff80086d0798>] rtnl_newlink+0x398/0x6a8
> [ 3370.029158] [<ffffff80086cee84>] rtnetlink_rcv_msg+0xe4/0x220
> [ 3370.034891] [<ffffff80086e274c>] netlink_rcv_skb+0xc4/0xf8
> [ 3370.040364] [<ffffff80086ced8c>] rtnetlink_rcv+0x2c/0x40
> [ 3370.045663] [<ffffff80086e1fe8>] netlink_unicast+0x160/0x238
> [ 3370.051309] [<ffffff80086e24b8>] netlink_sendmsg+0x2f0/0x358
> [ 3370.056956] [<ffffff80086a0070>] sock_sendmsg+0x18/0x30
> [ 3370.062168] [<ffffff80086a21cc>] ___sys_sendmsg+0x26c/0x280
> [ 3370.067728] [<ffffff80086a30ac>] __sys_sendmsg+0x44/0x88
> [ 3370.073027] [<ffffff80086a3100>] SyS_sendmsg+0x10/0x20
> [ 3370.078153] [<ffffff8008085e70>] el0_svc_naked+0x24/0x28

Nice fix and simplification!

Could you please forward-port this to branch rcu/dev of -rcu?

	git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git

							Thanx, Paul


> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Dennis Chen <dennis.chen@arm.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  kernel/rcu/tree.c | 30 +++++++++++++-----------------
>  kernel/rcu/tree.h | 18 ++++++++++++++++++
>  2 files changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 9a535a8..2923df3 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1235,15 +1235,16 @@ static void rcu_check_gp_kthread_starvation(struct rcu_state *rsp)
>  static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
>  {
>  	int cpu;
> +	unsigned long bit;
>  	unsigned long flags;
>  	struct rcu_node *rnp;
> 
>  	rcu_for_each_leaf_node(rsp, rnp) {
>  		raw_spin_lock_irqsave_rcu_node(rnp, flags);
>  		if (rnp->qsmask != 0) {
> -			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
> -				if (rnp->qsmask & (1UL << cpu))
> -					dump_cpu_task(rnp->grplo + cpu);
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
> +				if (rnp->qsmask & bit)
> +					dump_cpu_task(cpu);
>  		}
>  		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
>  	}
> @@ -1252,6 +1253,7 @@ static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
>  static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
>  {
>  	int cpu;
> +	unsigned long bit;
>  	long delta;
>  	unsigned long flags;
>  	unsigned long gpa;
> @@ -1284,10 +1286,9 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
>  		raw_spin_lock_irqsave_rcu_node(rnp, flags);
>  		ndetected += rcu_print_task_stall(rnp);
>  		if (rnp->qsmask != 0) {
> -			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
> -				if (rnp->qsmask & (1UL << cpu)) {
> -					print_cpu_stall_info(rsp,
> -							     rnp->grplo + cpu);
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
> +				if (rnp->qsmask & bit) {
> +					print_cpu_stall_info(rsp, cpu);
>  					ndetected++;
>  				}
>  		}
> @@ -2823,9 +2824,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
>  				continue;
>  			}
>  		}
> -		cpu = rnp->grplo;
> -		bit = 1;
> -		for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
> +		for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) {
>  			if ((rnp->qsmask & bit) != 0) {
>  				if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
>  					mask |= bit;
> @@ -3690,7 +3689,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
> 
>  		/* Each pass checks a CPU for identity, offline, and idle. */
>  		mask_ofl_test = 0;
> -		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++) {
> +		for_each_leaf_node_possible_cpu(rnp, cpu) {
>  			struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
>  			struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
> 
> @@ -3710,8 +3709,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
>  		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> 
>  		/* IPI the remaining CPUs for expedited quiescent state. */
> -		mask = 1;
> -		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +		for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  			if (!(mask_ofl_ipi & mask))
>  				continue;
>  retry_ipi:
> @@ -3774,8 +3772,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
>  		ndetected = 0;
>  		rcu_for_each_leaf_node(rsp, rnp) {
>  			ndetected = rcu_print_task_exp_stall(rnp);
> -			mask = 1;
> -			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  				struct rcu_data *rdp;
> 
>  				if (!(rnp->expmask & mask))
> @@ -3807,8 +3804,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
>  			pr_cont("\n");
>  		}
>  		rcu_for_each_leaf_node(rsp, rnp) {
> -			mask = 1;
> -			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  				if (!(rnp->expmask & mask))
>  					continue;
>  				dump_cpu_task(cpu);
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index df668c0..d823b8e 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -283,6 +283,24 @@ struct rcu_node {
>  	     (rnp) < &(rsp)->node[rcu_num_nodes]; (rnp)++)
> 
>  /*
> + * Iterate over all possible CPUs in a leaf RCU node.
> + */
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = rnp->grplo; \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask))
> +
> +/*
> + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> + * bit for comparison against rcu_node bitmasks.
> + */
> +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> +	for ((cpu) = rnp->grplo, (bit) = 1; \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask), \
> +		   (bit) = 1UL << (cpu - rnp->grplo))
> +
> +/*
>   * Union to allow "aggregate OR" operation on the need for a quiescent
>   * state by the normal and expedited grace periods.
>   */
> -- 
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1402277 — [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

FromMark Rutland <mark.rutland@arm.com>
Date2016-05-17 12:30 +0200
Subject[PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rzKqf-3kb-37@gated-at.bofh.it>
In reply to#1401720
In many cases in the RCU tree code, we iterate over the set of CPUs for
a leaf node described by rcu_node::grplo and rcu_node::grphi, checking
per-cpu data for each CPU in this range. However, if the set of possible
CPUs is sparse, some CPUs described in this range are not possible, and
thus no per-cpu region will have been allocated (or initialised) for
them by the generic percpu code.

Erroneous accesses to a per-cpu area for these !possible CPUs may fault
or may hit other data depending on the addressed generated when the
erroneous per cpu offset is applied. In practice, both cases have been
observed on arm64 hardware (the former being silent, but detectable with
additional patches).

To avoid issues resulting from this, we must iterate over the set of
*possible* cpus for a given leaf node. This patch adds new helpers to
enable this (also unifying and simplifying some related bitmask
manipulation logic), and moves the RCU tree code over to them.

Without this patch, running reboot at a shell can result in an oops
like:

[ 3369.075979] Unable to handle kernel paging request at virtual address ffffff8008b21b4c
[ 3369.083881] pgd = ffffffc3ecdda000
[ 3369.087270] [ffffff8008b21b4c] *pgd=00000083eca48003, *pud=00000083eca48003, *pmd=0000000000000000
[ 3369.096222] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[ 3369.101781] Modules linked in:
[ 3369.104825] CPU: 2 PID: 1817 Comm: NetworkManager Tainted: G        W       4.6.0+ #3
[ 3369.121239] task: ffffffc0fa13e000 ti: ffffffc3eb940000 task.ti: ffffffc3eb940000
[ 3369.128708] PC is at sync_rcu_exp_select_cpus+0x188/0x510
[ 3369.134094] LR is at sync_rcu_exp_select_cpus+0x104/0x510
[ 3369.139479] pc : [<ffffff80081109a8>] lr : [<ffffff8008110924>] pstate: 200001c5
[ 3369.146860] sp : ffffffc3eb9435a0
[ 3369.150162] x29: ffffffc3eb9435a0 x28: ffffff8008be4f88
[ 3369.155465] x27: ffffff8008b66c80 x26: ffffffc3eceb2600
[ 3369.160767] x25: 0000000000000001 x24: ffffff8008be4f88
[ 3369.166070] x23: ffffff8008b51c3c x22: ffffff8008b66c80
[ 3369.171371] x21: 0000000000000001 x20: ffffff8008b21b40
[ 3369.176673] x19: ffffff8008b66c80 x18: 0000000000000000
[ 3369.181975] x17: 0000007fa951a010 x16: ffffff80086a30f0
[ 3369.187278] x15: 0000007fa9505590 x14: 0000000000000000
[ 3369.192580] x13: ffffff8008b51000 x12: ffffffc3eb940000
[ 3369.197882] x11: 0000000000000006 x10: ffffff8008b51b78
[ 3369.203184] x9 : 0000000000000001 x8 : ffffff8008be4000
[ 3369.208486] x7 : ffffff8008b21b40 x6 : 0000000000001003
[ 3369.213788] x5 : 0000000000000000 x4 : ffffff8008b27280
[ 3369.219090] x3 : ffffff8008b21b4c x2 : 0000000000000001
[ 3369.224406] x1 : 0000000000000001 x0 : 0000000000000140
...
[ 3369.972257] [<ffffff80081109a8>] sync_rcu_exp_select_cpus+0x188/0x510
[ 3369.978685] [<ffffff80081128b4>] synchronize_rcu_expedited+0x64/0xa8
[ 3369.985026] [<ffffff80086b987c>] synchronize_net+0x24/0x30
[ 3369.990499] [<ffffff80086ddb54>] dev_deactivate_many+0x28c/0x298
[ 3369.996493] [<ffffff80086b6bb8>] __dev_close_many+0x60/0xd0
[ 3370.002052] [<ffffff80086b6d48>] __dev_close+0x28/0x40
[ 3370.007178] [<ffffff80086bf62c>] __dev_change_flags+0x8c/0x158
[ 3370.012999] [<ffffff80086bf718>] dev_change_flags+0x20/0x60
[ 3370.018558] [<ffffff80086cf7f0>] do_setlink+0x288/0x918
[ 3370.023771] [<ffffff80086d0798>] rtnl_newlink+0x398/0x6a8
[ 3370.029158] [<ffffff80086cee84>] rtnetlink_rcv_msg+0xe4/0x220
[ 3370.034891] [<ffffff80086e274c>] netlink_rcv_skb+0xc4/0xf8
[ 3370.040364] [<ffffff80086ced8c>] rtnetlink_rcv+0x2c/0x40
[ 3370.045663] [<ffffff80086e1fe8>] netlink_unicast+0x160/0x238
[ 3370.051309] [<ffffff80086e24b8>] netlink_sendmsg+0x2f0/0x358
[ 3370.056956] [<ffffff80086a0070>] sock_sendmsg+0x18/0x30
[ 3370.062168] [<ffffff80086a21cc>] ___sys_sendmsg+0x26c/0x280
[ 3370.067728] [<ffffff80086a30ac>] __sys_sendmsg+0x44/0x88
[ 3370.073027] [<ffffff80086a3100>] SyS_sendmsg+0x10/0x20
[ 3370.078153] [<ffffff8008085e70>] el0_svc_naked+0x24/0x28

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Dennis Chen <dennis.chen@arm.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/rcu/tree.c        | 19 +++++++++----------
 kernel/rcu/tree.h        | 18 ++++++++++++++++++
 kernel/rcu/tree_exp.h    | 12 ++++--------
 kernel/rcu/tree_plugin.h |  5 +++--
 4 files changed, 34 insertions(+), 20 deletions(-)

Since v1 [1]:
 * rebase to the -rcu rcu/dev branch.
 * replace all occurences missed by v1.
 * s/CPUS/CPUs/ in commit message. Gah.

Paul, I've given this a spin on arm64 and I've build-tested for x86.
Things look fine, though this hasn't seen a thorough beating.

Mark.

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index afdcb7b..65ee19e 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1279,15 +1279,16 @@ static void rcu_check_gp_kthread_starvation(struct rcu_state *rsp)
 static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
 {
 	int cpu;
+	unsigned long bit;
 	unsigned long flags;
 	struct rcu_node *rnp;
 
 	rcu_for_each_leaf_node(rsp, rnp) {
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 		if (rnp->qsmask != 0) {
-			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
-				if (rnp->qsmask & (1UL << cpu))
-					dump_cpu_task(rnp->grplo + cpu);
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
+				if (rnp->qsmask & bit)
+					dump_cpu_task(cpu);
 		}
 		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 	}
@@ -1316,6 +1317,7 @@ static void rcu_stall_kick_kthreads(struct rcu_state *rsp)
 static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
 {
 	int cpu;
+	unsigned long bit;
 	long delta;
 	unsigned long flags;
 	unsigned long gpa;
@@ -1353,10 +1355,9 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 		ndetected += rcu_print_task_stall(rnp);
 		if (rnp->qsmask != 0) {
-			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
-				if (rnp->qsmask & (1UL << cpu)) {
-					print_cpu_stall_info(rsp,
-							     rnp->grplo + cpu);
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
+				if (rnp->qsmask & bit) {
+					print_cpu_stall_info(rsp, cpu);
 					ndetected++;
 				}
 		}
@@ -2908,9 +2909,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
 				continue;
 			}
 		}
-		cpu = rnp->grplo;
-		bit = 1;
-		for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
+		for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) {
 			if ((rnp->qsmask & bit) != 0) {
 				if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
 					mask |= bit;
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index e3959f5..f73444f 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -281,6 +281,24 @@ struct rcu_node {
 	     (rnp) < &(rsp)->node[rcu_num_nodes]; (rnp)++)
 
 /*
+ * Iterate over all possible CPUs in a leaf RCU node.
+ */
+#define for_each_leaf_node_possible_cpu(rnp, cpu) \
+	for ((cpu) = rnp->grplo; \
+	     cpu <= rnp->grphi; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask))
+
+/*
+ * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
+ * bit for comparison against rcu_node bitmasks.
+ */
+#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
+	for ((cpu) = rnp->grplo, (bit) = 1; \
+	     cpu <= rnp->grphi; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask), \
+		   (bit) = 1UL << (cpu - rnp->grplo))
+
+/*
  * Union to allow "aggregate OR" operation on the need for a quiescent
  * state by the normal and expedited grace periods.
  */
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index 00a02a2..6655a44 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -356,7 +356,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 
 		/* Each pass checks a CPU for identity, offline, and idle. */
 		mask_ofl_test = 0;
-		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++) {
+		for_each_leaf_node_possible_cpu(rnp, cpu) {
 			struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
 			struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
 
@@ -376,8 +376,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
 		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 
 		/* IPI the remaining CPUs for expedited quiescent state. */
-		mask = 1;
-		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+		for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 			if (!(mask_ofl_ipi & mask))
 				continue;
 retry_ipi:
@@ -440,8 +439,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 		ndetected = 0;
 		rcu_for_each_leaf_node(rsp, rnp) {
 			ndetected += rcu_print_task_exp_stall(rnp);
-			mask = 1;
-			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 				struct rcu_data *rdp;
 
 				if (!(rnp->expmask & mask))
@@ -453,7 +451,6 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 					"o."[!!(rdp->grpmask & rnp->expmaskinit)],
 					"N."[!!(rdp->grpmask & rnp->expmaskinitnext)]);
 			}
-			mask <<= 1;
 		}
 		pr_cont(" } %lu jiffies s: %lu root: %#lx/%c\n",
 			jiffies - jiffies_start, rsp->expedited_sequence,
@@ -473,8 +470,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 			pr_cont("\n");
 		}
 		rcu_for_each_leaf_node(rsp, rnp) {
-			mask = 1;
-			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
+			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
 				if (!(rnp->expmask & mask))
 					continue;
 				dump_cpu_task(cpu);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 02a9197..fd6b0f7 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1157,6 +1157,7 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
 {
 	struct task_struct *t = rnp->boost_kthread_task;
 	unsigned long mask = rcu_rnp_online_cpus(rnp);
+	unsigned long bit;
 	cpumask_var_t cm;
 	int cpu;
 
@@ -1164,8 +1165,8 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
 		return;
 	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
 		return;
-	for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask >>= 1)
-		if ((mask & 0x1) && cpu != outgoingcpu)
+	for_each_leaf_node_cpu_bit(rnp, cpu, bit)
+		if ((mask & bit) && cpu != outgoingcpu)
 			cpumask_set_cpu(cpu, cm);
 	if (cpumask_weight(cm) == 0)
 		cpumask_setall(cm);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1402536 — Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-05-17 21:10 +0200
SubjectRe: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rzSxs-7q-21@gated-at.bofh.it>
In reply to#1402277
On Tue, May 17, 2016 at 11:22:10AM +0100, Mark Rutland wrote:
> In many cases in the RCU tree code, we iterate over the set of CPUs for
> a leaf node described by rcu_node::grplo and rcu_node::grphi, checking
> per-cpu data for each CPU in this range. However, if the set of possible
> CPUs is sparse, some CPUs described in this range are not possible, and
> thus no per-cpu region will have been allocated (or initialised) for
> them by the generic percpu code.
> 
> Erroneous accesses to a per-cpu area for these !possible CPUs may fault
> or may hit other data depending on the addressed generated when the
> erroneous per cpu offset is applied. In practice, both cases have been
> observed on arm64 hardware (the former being silent, but detectable with
> additional patches).
> 
> To avoid issues resulting from this, we must iterate over the set of
> *possible* cpus for a given leaf node. This patch adds new helpers to
> enable this (also unifying and simplifying some related bitmask
> manipulation logic), and moves the RCU tree code over to them.
> 
> Without this patch, running reboot at a shell can result in an oops
> like:

Very good, this one applies cleanly and I have queued it for review
and testing.

One question below, though.

							Thanx, Paul

> [ 3369.075979] Unable to handle kernel paging request at virtual address ffffff8008b21b4c
> [ 3369.083881] pgd = ffffffc3ecdda000
> [ 3369.087270] [ffffff8008b21b4c] *pgd=00000083eca48003, *pud=00000083eca48003, *pmd=0000000000000000
> [ 3369.096222] Internal error: Oops: 96000007 [#1] PREEMPT SMP
> [ 3369.101781] Modules linked in:
> [ 3369.104825] CPU: 2 PID: 1817 Comm: NetworkManager Tainted: G        W       4.6.0+ #3
> [ 3369.121239] task: ffffffc0fa13e000 ti: ffffffc3eb940000 task.ti: ffffffc3eb940000
> [ 3369.128708] PC is at sync_rcu_exp_select_cpus+0x188/0x510
> [ 3369.134094] LR is at sync_rcu_exp_select_cpus+0x104/0x510
> [ 3369.139479] pc : [<ffffff80081109a8>] lr : [<ffffff8008110924>] pstate: 200001c5
> [ 3369.146860] sp : ffffffc3eb9435a0
> [ 3369.150162] x29: ffffffc3eb9435a0 x28: ffffff8008be4f88
> [ 3369.155465] x27: ffffff8008b66c80 x26: ffffffc3eceb2600
> [ 3369.160767] x25: 0000000000000001 x24: ffffff8008be4f88
> [ 3369.166070] x23: ffffff8008b51c3c x22: ffffff8008b66c80
> [ 3369.171371] x21: 0000000000000001 x20: ffffff8008b21b40
> [ 3369.176673] x19: ffffff8008b66c80 x18: 0000000000000000
> [ 3369.181975] x17: 0000007fa951a010 x16: ffffff80086a30f0
> [ 3369.187278] x15: 0000007fa9505590 x14: 0000000000000000
> [ 3369.192580] x13: ffffff8008b51000 x12: ffffffc3eb940000
> [ 3369.197882] x11: 0000000000000006 x10: ffffff8008b51b78
> [ 3369.203184] x9 : 0000000000000001 x8 : ffffff8008be4000
> [ 3369.208486] x7 : ffffff8008b21b40 x6 : 0000000000001003
> [ 3369.213788] x5 : 0000000000000000 x4 : ffffff8008b27280
> [ 3369.219090] x3 : ffffff8008b21b4c x2 : 0000000000000001
> [ 3369.224406] x1 : 0000000000000001 x0 : 0000000000000140
> ...
> [ 3369.972257] [<ffffff80081109a8>] sync_rcu_exp_select_cpus+0x188/0x510
> [ 3369.978685] [<ffffff80081128b4>] synchronize_rcu_expedited+0x64/0xa8
> [ 3369.985026] [<ffffff80086b987c>] synchronize_net+0x24/0x30
> [ 3369.990499] [<ffffff80086ddb54>] dev_deactivate_many+0x28c/0x298
> [ 3369.996493] [<ffffff80086b6bb8>] __dev_close_many+0x60/0xd0
> [ 3370.002052] [<ffffff80086b6d48>] __dev_close+0x28/0x40
> [ 3370.007178] [<ffffff80086bf62c>] __dev_change_flags+0x8c/0x158
> [ 3370.012999] [<ffffff80086bf718>] dev_change_flags+0x20/0x60
> [ 3370.018558] [<ffffff80086cf7f0>] do_setlink+0x288/0x918
> [ 3370.023771] [<ffffff80086d0798>] rtnl_newlink+0x398/0x6a8
> [ 3370.029158] [<ffffff80086cee84>] rtnetlink_rcv_msg+0xe4/0x220
> [ 3370.034891] [<ffffff80086e274c>] netlink_rcv_skb+0xc4/0xf8
> [ 3370.040364] [<ffffff80086ced8c>] rtnetlink_rcv+0x2c/0x40
> [ 3370.045663] [<ffffff80086e1fe8>] netlink_unicast+0x160/0x238
> [ 3370.051309] [<ffffff80086e24b8>] netlink_sendmsg+0x2f0/0x358
> [ 3370.056956] [<ffffff80086a0070>] sock_sendmsg+0x18/0x30
> [ 3370.062168] [<ffffff80086a21cc>] ___sys_sendmsg+0x26c/0x280
> [ 3370.067728] [<ffffff80086a30ac>] __sys_sendmsg+0x44/0x88
> [ 3370.073027] [<ffffff80086a3100>] SyS_sendmsg+0x10/0x20
> [ 3370.078153] [<ffffff8008085e70>] el0_svc_naked+0x24/0x28
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Dennis Chen <dennis.chen@arm.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  kernel/rcu/tree.c        | 19 +++++++++----------
>  kernel/rcu/tree.h        | 18 ++++++++++++++++++
>  kernel/rcu/tree_exp.h    | 12 ++++--------
>  kernel/rcu/tree_plugin.h |  5 +++--
>  4 files changed, 34 insertions(+), 20 deletions(-)
> 
> Since v1 [1]:
>  * rebase to the -rcu rcu/dev branch.
>  * replace all occurences missed by v1.
>  * s/CPUS/CPUs/ in commit message. Gah.
> 
> Paul, I've given this a spin on arm64 and I've build-tested for x86.
> Things look fine, though this hasn't seen a thorough beating.
> 
> Mark.
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index afdcb7b..65ee19e 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1279,15 +1279,16 @@ static void rcu_check_gp_kthread_starvation(struct rcu_state *rsp)
>  static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
>  {
>  	int cpu;
> +	unsigned long bit;
>  	unsigned long flags;
>  	struct rcu_node *rnp;
> 
>  	rcu_for_each_leaf_node(rsp, rnp) {
>  		raw_spin_lock_irqsave_rcu_node(rnp, flags);
>  		if (rnp->qsmask != 0) {
> -			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
> -				if (rnp->qsmask & (1UL << cpu))
> -					dump_cpu_task(rnp->grplo + cpu);
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
> +				if (rnp->qsmask & bit)
> +					dump_cpu_task(cpu);
>  		}
>  		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
>  	}
> @@ -1316,6 +1317,7 @@ static void rcu_stall_kick_kthreads(struct rcu_state *rsp)
>  static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
>  {
>  	int cpu;
> +	unsigned long bit;
>  	long delta;
>  	unsigned long flags;
>  	unsigned long gpa;
> @@ -1353,10 +1355,9 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)
>  		raw_spin_lock_irqsave_rcu_node(rnp, flags);
>  		ndetected += rcu_print_task_stall(rnp);
>  		if (rnp->qsmask != 0) {
> -			for (cpu = 0; cpu <= rnp->grphi - rnp->grplo; cpu++)
> -				if (rnp->qsmask & (1UL << cpu)) {
> -					print_cpu_stall_info(rsp,
> -							     rnp->grplo + cpu);
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
> +				if (rnp->qsmask & bit) {
> +					print_cpu_stall_info(rsp, cpu);
>  					ndetected++;
>  				}
>  		}
> @@ -2908,9 +2909,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
>  				continue;
>  			}
>  		}
> -		cpu = rnp->grplo;
> -		bit = 1;
> -		for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
> +		for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) {
>  			if ((rnp->qsmask & bit) != 0) {
>  				if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
>  					mask |= bit;
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index e3959f5..f73444f 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -281,6 +281,24 @@ struct rcu_node {
>  	     (rnp) < &(rsp)->node[rcu_num_nodes]; (rnp)++)
> 
>  /*
> + * Iterate over all possible CPUs in a leaf RCU node.
> + */
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = rnp->grplo; \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask))

What if the rnp->grplo corresponds to a non-existent CPU?  I admit that
this is perhaps unlikely given the usual power-of-two defaults and
pwoer-of-two computer-system structure, but...

Would something like this handle that possibility?

+#define for_each_leaf_node_possible_cpu(rnp, cpu) \
+	for ((cpu) = cpumask_next(rnp->grplo - 1, cpu_possible_mask); \
+	     cpu <= rnp->grphi; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask))

Or maybe like this, with less duplicated code but very strange style:

+#define for_each_leaf_node_possible_cpu(rnp, cpu) \
+	for ((cpu) = rnp->grplo - 1; \
+	     cpu = cpumask_next((cpu), cpu_possible_mask), cpu <= rnp->grphi; 1)

The first one is probably far better, assuming that it works, but I could
not resist inflicting the second one on you.  ;-)

> +
> +/*
> + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> + * bit for comparison against rcu_node bitmasks.
> + */
> +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> +	for ((cpu) = rnp->grplo, (bit) = 1; \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask), \
> +		   (bit) = 1UL << (cpu - rnp->grplo))

Same question here.

> +
> +/*
>   * Union to allow "aggregate OR" operation on the need for a quiescent
>   * state by the normal and expedited grace periods.
>   */
> diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
> index 00a02a2..6655a44 100644
> --- a/kernel/rcu/tree_exp.h
> +++ b/kernel/rcu/tree_exp.h
> @@ -356,7 +356,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
> 
>  		/* Each pass checks a CPU for identity, offline, and idle. */
>  		mask_ofl_test = 0;
> -		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++) {
> +		for_each_leaf_node_possible_cpu(rnp, cpu) {
>  			struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
>  			struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
> 
> @@ -376,8 +376,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,
>  		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> 
>  		/* IPI the remaining CPUs for expedited quiescent state. */
> -		mask = 1;
> -		for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +		for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  			if (!(mask_ofl_ipi & mask))
>  				continue;
>  retry_ipi:
> @@ -440,8 +439,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
>  		ndetected = 0;
>  		rcu_for_each_leaf_node(rsp, rnp) {
>  			ndetected += rcu_print_task_exp_stall(rnp);
> -			mask = 1;
> -			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  				struct rcu_data *rdp;
> 
>  				if (!(rnp->expmask & mask))
> @@ -453,7 +451,6 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
>  					"o."[!!(rdp->grpmask & rnp->expmaskinit)],
>  					"N."[!!(rdp->grpmask & rnp->expmaskinitnext)]);
>  			}
> -			mask <<= 1;
>  		}
>  		pr_cont(" } %lu jiffies s: %lu root: %#lx/%c\n",
>  			jiffies - jiffies_start, rsp->expedited_sequence,
> @@ -473,8 +470,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
>  			pr_cont("\n");
>  		}
>  		rcu_for_each_leaf_node(rsp, rnp) {
> -			mask = 1;
> -			for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<= 1) {
> +			for_each_leaf_node_possible_cpu_bit(rnp, cpu, mask) {
>  				if (!(rnp->expmask & mask))
>  					continue;
>  				dump_cpu_task(cpu);
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 02a9197..fd6b0f7 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1157,6 +1157,7 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
>  {
>  	struct task_struct *t = rnp->boost_kthread_task;
>  	unsigned long mask = rcu_rnp_online_cpus(rnp);
> +	unsigned long bit;
>  	cpumask_var_t cm;
>  	int cpu;
> 
> @@ -1164,8 +1165,8 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
>  		return;
>  	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
>  		return;
> -	for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask >>= 1)
> -		if ((mask & 0x1) && cpu != outgoingcpu)
> +	for_each_leaf_node_cpu_bit(rnp, cpu, bit)
> +		if ((mask & bit) && cpu != outgoingcpu)
>  			cpumask_set_cpu(cpu, cm);
>  	if (cpumask_weight(cm) == 0)
>  		cpumask_setall(cm);
> -- 
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1402731 — Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

FromMark Rutland <mark.rutland@arm.com>
Date2016-05-18 08:10 +0200
SubjectRe: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rA2Q9-6EU-1@gated-at.bofh.it>
In reply to#1402536
On Tue, May 17, 2016 at 12:01:06PM -0700, Paul E. McKenney wrote:
> On Tue, May 17, 2016 at 11:22:10AM +0100, Mark Rutland wrote:
> >  /*
> > + * Iterate over all possible CPUs in a leaf RCU node.
> > + */
> > +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> > +	for ((cpu) = rnp->grplo; \
> > +	     cpu <= rnp->grphi; \
> > +	     cpu = cpumask_next((cpu), cpu_possible_mask))
> 
> What if the rnp->grplo corresponds to a non-existent CPU?

Good point, I had evidently not considered that.

> Would something like this handle that possibility?
> 
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = cpumask_next(rnp->grplo - 1, cpu_possible_mask); \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask))
> 
> Or maybe like this, with less duplicated code but very strange style:
> 
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = rnp->grplo - 1; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask), cpu <= rnp->grphi; 1)
> 
> The first one is probably far better, assuming that it works, but I could
> not resist inflicting the second one on you.  ;-)

:)

Those both look like they should work, I'll fold the former in.

> > +/*
> > + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> > + * bit for comparison against rcu_node bitmasks.
> > + */
> > +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> > +	for ((cpu) = rnp->grplo, (bit) = 1; \
> > +	     cpu <= rnp->grphi; \
> > +	     cpu = cpumask_next((cpu), cpu_possible_mask), \
> > +		   (bit) = 1UL << (cpu - rnp->grplo))
> 
> Same question here.

Likewise.

I'll also see about fixing the build issue you spotted in the other reply; that
appears to be a typo (missing 'possible_' in the macro invocation).

I'm away from my development machine at the moment, so that may not appear
until next week.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1402909 — Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

FromArnd Bergmann <arnd@arndb.de>
Date2016-05-18 14:10 +0200
SubjectRe: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rA8sy-1WF-23@gated-at.bofh.it>
In reply to#1402536
On Tuesday 17 May 2016 17:12:51 Paul E. McKenney wrote:
> And some build errors:
> 
> In file included from /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:4209:0:
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree_plugin.h: In function ‘rcu_boost_kthread_setaffinity’:
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree_plugin.h:1168:2: error: implicit declaration of function ‘for_each_leaf_node_cpu_bit’ [-Werror=implicit-function-declaration]
>   for_each_leaf_node_cpu_bit(rnp, cpu, bit)
>   ^
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree_plugin.h:1169:3: error: expected ‘;’ before ‘if’
>    if ((mask & bit) && cpu != outgoingcpu)
>    ^
> /home/paulmck/public_git/linux-rcu/kernel/rcu/tree_plugin.h:1159:16: warning: unused variable ‘mask’ [-Wunused-variable]
>   unsigned long mask = rcu_rnp_online_cpus(rnp);
>                 ^
> 
> Please see below for the .config.
> 
> I have dropped the patch from my tree, looking forward to getting an
> update that fixes the build errors.
> 

It's the missing "possible_" that Mark mentioned in his reply on Friday.

Please fold the fixup below into the patch if you want to get it to build.

Signed-off-by: Arnd Bergmann <arnd@arndb.de

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index fd6b0f701bed..bb137b0ef6f3 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1165,7 +1165,7 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
 		return;
 	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
 		return;
-	for_each_leaf_node_cpu_bit(rnp, cpu, bit)
+	for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
 		if ((mask & bit) && cpu != outgoingcpu)
 			cpumask_set_cpu(cpu, cm);
 	if (cpumask_weight(cm) == 0)

[toc] | [prev] | [next] | [standalone]

#1403210 — Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

FromMark Rutland <mark.rutland@arm.com>
Date2016-05-18 20:20 +0200
SubjectRe: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rAeeC-5Ah-5@gated-at.bofh.it>
In reply to#1402909
On Wed, May 18, 2016 at 02:02:36PM +0200, Arnd Bergmann wrote:
> It's the missing "possible_" that Mark mentioned in his reply on Friday.

Actually, that was this morning. My VM on my laptop had a stale date due to
suspend/resume of the host. :/

I should be back at a real computer by Friday, and can respin the patch to fix
the issue Andrey pointed out.

Thanks for the fixup, and sorry for the confusion!

Mark.

> Please fold the fixup below into the patch if you want to get it to build.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de
> 
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index fd6b0f701bed..bb137b0ef6f3 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1165,7 +1165,7 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
>  		return;
>  	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
>  		return;
> -	for_each_leaf_node_cpu_bit(rnp, cpu, bit)
> +	for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
>  		if ((mask & bit) && cpu != outgoingcpu)
>  			cpumask_set_cpu(cpu, cm);
>  	if (cpumask_weight(cm) == 0)
>

[toc] | [prev] | [next] | [standalone]

#1403223 — Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-05-18 20:50 +0200
SubjectRe: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs
Message-ID<rAeHE-5LI-17@gated-at.bofh.it>
In reply to#1403210
On Wed, May 18, 2016 at 07:15:09PM +0100, Mark Rutland wrote:
> On Wed, May 18, 2016 at 02:02:36PM +0200, Arnd Bergmann wrote:
> > It's the missing "possible_" that Mark mentioned in his reply on Friday.
> 
> Actually, that was this morning. My VM on my laptop had a stale date due to
> suspend/resume of the host. :/
> 
> I should be back at a real computer by Friday, and can respin the patch to fix
> the issue Andrey pointed out.
> 
> Thanks for the fixup, and sorry for the confusion!
> 
> Mark.
> 
> > Please fold the fixup below into the patch if you want to get it to build.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de

Thank you both!

Looking forward to the respin that covers the various issues that have
been located.

							Thanx, Paul

> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index fd6b0f701bed..bb137b0ef6f3 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -1165,7 +1165,7 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
> >  		return;
> >  	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
> >  		return;
> > -	for_each_leaf_node_cpu_bit(rnp, cpu, bit)
> > +	for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit)
> >  		if ((mask & bit) && cpu != outgoingcpu)
> >  			cpumask_set_cpu(cpu, cm);
> >  	if (cpumask_weight(cm) == 0)
> > 
>

[toc] | [prev] | [next] | [standalone]

#1403070

FromAndrey Ryabinin <ryabinin.a.a@gmail.com>
Date2016-05-18 17:20 +0200
Message-ID<rAbqp-3On-1@gated-at.bofh.it>
In reply to#1401622
2016-05-16 19:48 GMT+03:00 Mark Rutland <mark.rutland@arm.com>:

>  /*
> + * Iterate over all possible CPUs in a leaf RCU node.
> + */
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +       for ((cpu) = rnp->grplo; \
> +            cpu <= rnp->grphi; \
> +            cpu = cpumask_next((cpu), cpu_possible_mask))
> +
> +/*
> + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> + * bit for comparison against rcu_node bitmasks.
> + */
> +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> +       for ((cpu) = rnp->grplo, (bit) = 1; \
> +            cpu <= rnp->grphi; \
> +            cpu = cpumask_next((cpu), cpu_possible_mask), \
> +                  (bit) = 1UL << (cpu - rnp->grplo))
> +


[    0.163652] UBSAN: Undefined behaviour in ../kernel/rcu/tree.c:2912:3
[    0.164000] shift exponent 64 is too large for 64-bit type 'long
unsigned int'

[toc] | [prev] | [next] | [standalone]

#1403205

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-05-18 20:10 +0200
Message-ID<rAe4W-5wO-11@gated-at.bofh.it>
In reply to#1403070
On Wed, May 18, 2016 at 06:15:23PM +0300, Andrey Ryabinin wrote:
> 2016-05-16 19:48 GMT+03:00 Mark Rutland <mark.rutland@arm.com>:
> 
> >  /*
> > + * Iterate over all possible CPUs in a leaf RCU node.
> > + */
> > +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> > +       for ((cpu) = rnp->grplo; \
> > +            cpu <= rnp->grphi; \
> > +            cpu = cpumask_next((cpu), cpu_possible_mask))
> > +
> > +/*
> > + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> > + * bit for comparison against rcu_node bitmasks.
> > + */
> > +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> > +       for ((cpu) = rnp->grplo, (bit) = 1; \
> > +            cpu <= rnp->grphi; \
> > +            cpu = cpumask_next((cpu), cpu_possible_mask), \
> > +                  (bit) = 1UL << (cpu - rnp->grplo))
> > +
> 
> [    0.163652] UBSAN: Undefined behaviour in ../kernel/rcu/tree.c:2912:3
> [    0.164000] shift exponent 64 is too large for 64-bit type 'long
> unsigned int'

Ah, dead value, but can happen nevertheless.  One fix is to prevent the
assignment to bit when cpu > rnp->grphi.

Any ideas for a better fix?  And isn't there some combination of
signedness that makes shifting all the bits out of the value defined
to zero?  Or is that only for right shifts?

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1403214

FromMark Rutland <mark.rutland@arm.com>
Date2016-05-18 20:40 +0200
Message-ID<rAexY-5ID-13@gated-at.bofh.it>
In reply to#1403205
On Wed, May 18, 2016 at 11:01:53AM -0700, Paul E. McKenney wrote:
> On Wed, May 18, 2016 at 06:15:23PM +0300, Andrey Ryabinin wrote:
> > 2016-05-16 19:48 GMT+03:00 Mark Rutland <mark.rutland@arm.com>:
> > 
> > >  /*
> > > + * Iterate over all possible CPUs in a leaf RCU node.
> > > + */
> > > +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> > > +       for ((cpu) = rnp->grplo; \
> > > +            cpu <= rnp->grphi; \
> > > +            cpu = cpumask_next((cpu), cpu_possible_mask))
> > > +
> > > +/*
> > > + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> > > + * bit for comparison against rcu_node bitmasks.
> > > + */
> > > +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> > > +       for ((cpu) = rnp->grplo, (bit) = 1; \
> > > +            cpu <= rnp->grphi; \
> > > +            cpu = cpumask_next((cpu), cpu_possible_mask), \
> > > +                  (bit) = 1UL << (cpu - rnp->grplo))
> > > +
> > 
> > [    0.163652] UBSAN: Undefined behaviour in ../kernel/rcu/tree.c:2912:3
> > [    0.164000] shift exponent 64 is too large for 64-bit type 'long
> > unsigned int'
> 
> Ah, dead value, but can happen nevertheless.  One fix is to prevent the
> assignment to bit when cpu > rnp->grphi.
> 
> Any ideas for a better fix?  And isn't there some combination of
> signedness that makes shifting all the bits out of the value defined
> to zero?  Or is that only for right shifts?

We could add a (leaf/rcu)_node_cpu_mask(rnp, cpu) macro, and only use that in
the body of the loop. That would avoid the stale value and would be useful in a
couple of additional places.

If that makes sense to you, I can respin the patch with that.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1403225

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-05-18 20:50 +0200
Message-ID<rAeHE-5LI-19@gated-at.bofh.it>
In reply to#1403214
On Wed, May 18, 2016 at 07:30:41PM +0100, Mark Rutland wrote:
> On Wed, May 18, 2016 at 11:01:53AM -0700, Paul E. McKenney wrote:
> > On Wed, May 18, 2016 at 06:15:23PM +0300, Andrey Ryabinin wrote:
> > > 2016-05-16 19:48 GMT+03:00 Mark Rutland <mark.rutland@arm.com>:
> > > 
> > > >  /*
> > > > + * Iterate over all possible CPUs in a leaf RCU node.
> > > > + */
> > > > +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> > > > +       for ((cpu) = rnp->grplo; \
> > > > +            cpu <= rnp->grphi; \
> > > > +            cpu = cpumask_next((cpu), cpu_possible_mask))
> > > > +
> > > > +/*
> > > > + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> > > > + * bit for comparison against rcu_node bitmasks.
> > > > + */
> > > > +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> > > > +       for ((cpu) = rnp->grplo, (bit) = 1; \
> > > > +            cpu <= rnp->grphi; \
> > > > +            cpu = cpumask_next((cpu), cpu_possible_mask), \
> > > > +                  (bit) = 1UL << (cpu - rnp->grplo))
> > > > +
> > > 
> > > [    0.163652] UBSAN: Undefined behaviour in ../kernel/rcu/tree.c:2912:3
> > > [    0.164000] shift exponent 64 is too large for 64-bit type 'long
> > > unsigned int'
> > 
> > Ah, dead value, but can happen nevertheless.  One fix is to prevent the
> > assignment to bit when cpu > rnp->grphi.
> > 
> > Any ideas for a better fix?  And isn't there some combination of
> > signedness that makes shifting all the bits out of the value defined
> > to zero?  Or is that only for right shifts?
> 
> We could add a (leaf/rcu)_node_cpu_mask(rnp, cpu) macro, and only use that in
> the body of the loop. That would avoid the stale value and would be useful in a
> couple of additional places.
> 
> If that makes sense to you, I can respin the patch with that.

Please try it and then let's see what it looks like.

							Thanx, Paul

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web