Skip to contentSkip to conversation

rcu classic: update qlen when cpu offline

PATCH Applied Lai Jiangshan 3 participants 3 messages Jun 26, 2008 – Jul 13, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rcu classic: update qlen when cpu offline

When callbacks are moved from offline cpu to this cpu, the qlen field of this rdp should be updated.

---

kernel/rcuclassic.c +4 -0
index f4ffbd0..42aead6 100644
@@ -373,6 +373,10 @@ static void __rcu_offline_cpu(struct rcu_data *this_rdp,
 	rcu_move_batch(this_rdp, rdp->donelist, rdp->donetail);
 	rcu_move_batch(this_rdp, rdp->curlist, rdp->curtail);
 	rcu_move_batch(this_rdp, rdp->nxtlist, rdp->nxttail);
+
+	local_irq_disable();
+	this_rdp->qlen += rdp->qlen;
+	local_irq_enable();
 }
 
 static void rcu_offline_cpu(int cpu)
Reviewed-by on v1 1/1
#1 lore raw
Quoted 3 lines · Lai Jiangshan

When callbacks are moved from offline cpu to this cpu, the qlen field of this rdp should be updated.

Good catch!!!

The effect of this bug would be for force_quiescent_state() to be invoked when it should not and vice versa -- wasting cycles in the first case and letting RCU callbacks remain piled up in the second case. The bug is thus "benign" in that it does not result in premature grace-period termination, but should of course be fixed nonetheless.

Preemption is disabled by the caller's get_cpu_var(), so we are guaranteed to remain on the same CPU, as required. The local_irq_disable() is indeed needed, otherwise, an interrupt might invoke call_rcu() or call_rcu_bh(), which could cause that interrupt's increment of ->qlen to be lost.

So this patch looks correct to me. Good job, Jiangshan!!!

Ingo, would you be willing to add this patch to tip/core/rcu?

Quoted 19 lines

---

kernel/rcuclassic.c +4 -0
index f4ffbd0..42aead6 100644
@@ -373,6 +373,10 @@ static void __rcu_offline_cpu(struct rcu_data *this_rdp,
 	rcu_move_batch(this_rdp, rdp->donelist, rdp->donetail);
 	rcu_move_batch(this_rdp, rdp->curlist, rdp->curtail);
 	rcu_move_batch(this_rdp, rdp->nxtlist, rdp->nxttail);
+
+	local_irq_disable();
+	this_rdp->qlen += rdp->qlen;
+	local_irq_enable();
 }
 
 static void rcu_offline_cpu(int cpu)
Applied
#2 lore raw
Quoted 18 lines · Paul E. McKenney
Quoted 3 lines · Lai Jiangshan

When callbacks are moved from offline cpu to this cpu, the qlen field of this rdp should be updated.

Good catch!!!

The effect of this bug would be for force_quiescent_state() to be invoked when it should not and vice versa -- wasting cycles in the first case and letting RCU callbacks remain piled up in the second case. The bug is thus "benign" in that it does not result in premature grace-period termination, but should of course be fixed nonetheless.

Preemption is disabled by the caller's get_cpu_var(), so we are guaranteed to remain on the same CPU, as required. The local_irq_disable() is indeed needed, otherwise, an interrupt might invoke call_rcu() or call_rcu_bh(), which could cause that interrupt's increment of ->qlen to be lost.

Quoted 5 lines

So this patch looks correct to me. Good job, Jiangshan!!!

Ingo, would you be willing to add this patch to tip/core/rcu?

thanks, applied to tip/core/rcu. (sorry about the delay!)

Ingo