ksoftirqd: Enable IRQs and call cond_resched() before poking RCU

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

---
Changes since v1:
I mixed up the kernel versions I was patching against, sorry!

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644

kernel/softirq.c +5 -0

+
+		preempt_disable();
+		rcu_note_context_switch();
+		preempt_enable();
+
 		return;
 	}
 	local_irq_enable();

Signature · 1 line

2.1.1

Quoted 30 lines · Calvin Owens

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

---
Changes since v1:
I mixed up the kernel versions I was patching against, sorry!

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 660

Quoted 6 lines

kernel/softirq.c +0 -1

@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
 		cond_resched();

If this is for 3.20, we can just replace cond_resched() with cond_resched_rcu_qs(), and get rid of the direct call to rcu_note_context_switch(). This has the benefit of avoiding needless rcu_note_context_switch() overhead if cond_resched() actually did a reschedule.

But don't try it in 3.19 or earlier. ;-)

Quoted 44 lines · Paul E. McKenney

Quoted 30 lines · Calvin Owens

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

---
Changes since v1:
I mixed up the kernel versions I was patching against, sorry!

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 660

Quoted 6 lines

kernel/softirq.c +0 -1

@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
 		cond_resched();

If this is for 3.20, we can just replace cond_resched() with cond_resched_rcu_qs(), and get rid of the direct call to rcu_note_context_switch(). This has the benefit of avoiding needless rcu_note_context_switch() overhead if cond_resched() actually did a reschedule.

Quoted 1 line

But don't try it in 3.19 or earlier. ;-)

As in the following for 3.20. Does this version work for you?

Thanx, Paul

------------------------------------------------------------------------

ksoftirqd: Enable IRQs and call cond_resched() before poking RCU

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem, but the new cond_resched_rcu_qs() provides shorter code and avoids double RCU notification in the case where cond_resched() really did a context switch.

Signed-off-by Calvin Owens <[email protected]>

[ paulmck: Substituted shiny new cond_resched_rcu_qs() primitive. ]

Signed-off-by Paul E. McKenney <[email protected]>

kernel/softirq.c +1 -2

index 501baa9ac1be..8cdb98847c7b 100644
@@ -656,9 +656,8 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
-		cond_resched();
+		cond_resched_rcu_qs();
 		return;
 	}
 	local_irq_enable();

Quoted 45 lines · Paul E. McKenney

Quoted 44 lines · Paul E. McKenney

Quoted 30 lines · Calvin Owens

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

---
Changes since v1:
I mixed up the kernel versions I was patching against, sorry!

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 660

Quoted 6 lines

kernel/softirq.c +0 -1

@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
 		cond_resched();

If this is for 3.20, we can just replace cond_resched() with cond_resched_rcu_qs(), and get rid of the direct call to rcu_note_context_switch(). This has the benefit of avoiding needless rcu_note_context_switch() overhead if cond_resched() actually did a reschedule.

Quoted 3 lines

Quoted 1 line

But don't try it in 3.19 or earlier. ;-)

As in the following for 3.20. Does this version work for you?

That's great, thanks :)

Should this go to stable as well? It is technically a regression, albeit a rather long-standing one.

My original scenario was a bit contrived, but I tested this on some real loads and it makes a difference: on a heavily loaded Proxygen server, the aggregate softirq CPU usage decreases by roughly 10% (relative) given the same amount of traffic with the patch. It also produces statistically significant performance wins at higher loads on
webservers: about a 1% reduction in overall CPU utilization and improved latency metrics.

Quoted 50 lines · Calvin Owens

Quoted 48 lines · Paul E. McKenney

Quoted 44 lines · Paul E. McKenney

Quoted 30 lines · Calvin Owens

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

---
Changes since v1:
I mixed up the kernel versions I was patching against, sorry!

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 660

Quoted 6 lines

kernel/softirq.c +0 -1

@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
 		cond_resched();

If this is for 3.20, we can just replace cond_resched() with cond_resched_rcu_qs(), and get rid of the direct call to rcu_note_context_switch(). This has the benefit of avoiding needless rcu_note_context_switch() overhead if cond_resched() actually did a reschedule.

Quoted 1 line

But don't try it in 3.19 or earlier. ;-)

As in the following for 3.20. Does this version work for you?

Quoted 4 lines

That's great, thanks :)

Should this go to stable as well? It is technically a regression, albeit a rather long-standing one.

Your original patch could go into stable, but my updated version depends on functionality not present before 3.20.

Quoted 7 lines

My original scenario was a bit contrived, but I tested this on some real loads and it makes a difference: on a heavily loaded Proxygen server, the aggregate softirq CPU usage decreases by roughly 10% (relative) given the same amount of traffic with the patch. It also produces statistically significant performance wins at higher loads on
webservers: about a 1% reduction in overall CPU utilization and improved latency metrics.

OK, good to know. I have added this information to the commit log, please see below for updated commit.

Thanx, Paul

------------------------------------------------------------------------

ksoftirqd: Enable IRQs and call cond_resched() before poking RCU

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%. On a heavily loaded Proxygen server, the aggregate softirq CPU usage decreases by roughly 10% (relative) given the same amount of traffic with the patch. It also produces statistically significant performance wins at higher loads on webservers: about a 1% reduction in overall CPU utilization and improved latency metrics.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem, but the new cond_resched_rcu_qs() provides shorter code and avoids double RCU notification in the case where cond_resched() really did a context switch.

Signed-off-by Calvin Owens <[email protected]>

[ paulmck: Substituted shiny new cond_resched_rcu_qs() primitive. ]

Signed-off-by Paul E. McKenney <[email protected]>

[ paulmck: Added Calvin's measurements on Proxygen server and webservers. ]

kernel/softirq.c +1 -2

index 501baa9ac1be..8cdb98847c7b 100644
@@ -656,9 +656,8 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch();
 		local_irq_enable();
-		cond_resched();
+		cond_resched_rcu_qs();
 		return;
 	}
 	local_irq_enable();

Quoted 5 lines · Paul E. McKenney

Quoted 2 lines

Should this go to stable as well? It is technically a regression, albeit a rather long-standing one.

Your original patch could go into stable, but my updated version depends on functionality not present before 3.20.

Right. I'll wait until 3.20-rc1, and then I'll send the original to stable with a reference to the commit.

Quoted 9 lines · Calvin Owens

Quoted 5 lines · Paul E. McKenney

Quoted 2 lines

Should this go to stable as well? It is technically a regression, albeit a rather long-standing one.

Your original patch could go into stable, but my updated version depends on functionality not present before 3.20.

Right. I'll wait until 3.20-rc1, and then I'll send the original to stable with a reference to the commit.

We better do that in two steps:

1) The original patch tagged for stable
2) Pauls version as a delta patch

Can one of you please provide such a series?

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

Cc [email protected]

---
This version includes the "cpu" argument to rcu_note_context_switch() in order to apply cleanly to stable kernels. It will need to be removed to apply to 3.18+ and 3.19 (upstream commit 38200cf2 removed the argument).

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +5 -1

index 501baa9..9e787d8 100644
@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.
 		 */
 		__do_softirq();
-		rcu_note_context_switch(cpu);
 		local_irq_enable();
 		cond_resched();
+
+		preempt_disable();
+		rcu_note_context_switch(cpu);
+		preempt_enable();
+
 		return;
 	}
 	local_irq_enable();

Signature · 1 line

2.1.1

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

Cc [email protected]

---
This version includes the "cpu" argument to rcu_note_context_switch() in order to apply cleanly to stable kernels. It will need to be removed to apply to 3.18+ and 3.19 (upstream commit 38200cf2 removed the argument).

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644
@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.

Quoted 33 lines · Calvin Owens

While debugging an issue with excessive softirq usage, I encountered the following note in commit 3e339b5dae24a706 ("softirq: Use hotplug thread infrastructure"):

[ paulmck: Call rcu_note_context_switch() with interrupts enabled. ]

...but despite this note, the patch still calls RCU with IRQs disabled.

This seemingly innocuous change caused a significant regression in softirq CPU usage on the sending side of a large TCP transfer (~1 GB/s): when introducing 0.01% packet loss, the softirq usage would jump to around 25%, spiking as high as 50%. Before the change, the usage would never exceed 5%.

Moving the call to rcu_note_context_switch() after the cond_sched() call, as it was originally before the hotplug patch, completely eliminated this problem.

Signed-off-by Calvin Owens <[email protected]>

Cc [email protected]

---
This version includes the "cpu" argument to rcu_note_context_switch() in order to apply cleanly to stable kernels. It will need to be removed to apply to 3.18+ and 3.19 (upstream commit 38200cf2 removed the argument).

 kernel/softirq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644
@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 665

Quoted 10 lines

kernel/softirq.c +5 -1

 		 */
 		__do_softirq();
-		rcu_note_context_switch(cpu);
 		local_irq_enable();
 		cond_resched();
+
+		preempt_disable();
+		rcu_note_context_switch(cpu);
+		preempt_enable();
+

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

	__preempt_count_sub(PREEMPT_ACTIVE);

Quoted 27 lines · Thomas Gleixner

Quoted 6 lines

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644
@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 665

Quoted 10 lines

kernel/softirq.c +5 -1

 		 */
 		__do_softirq();
-		rcu_note_context_switch(cpu);
 		local_irq_enable();
 		cond_resched();
+
+		preempt_disable();
+		rcu_note_context_switch(cpu);
+		preempt_enable();
+

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

Quoted 1 line

	__preempt_count_sub(PREEMPT_ACTIVE);

I agree that if should_resched() returns true as assumed above, then there is no point to invoking rcu_note_context_switch(). However, the case that this code applies to is when should_resched() returns false, but RCU is waiting for a quiescent state from the current CPU. In that case, cond_resched() won't do anything for RCU, and we do need the rcu_note_context_switch().

Quoted 29 lines · Paul E. McKenney

Quoted 27 lines · Thomas Gleixner

Quoted 6 lines

kernel/softirq.c +0 -0

index 501baa9..9e787d8 100644
@@ -656,9 +656,13 @@ static void run_ksoftirqd(unsigned int cpu)
 		 * in the task stack here.

kernel/softirq.c static void run_ksoftirqd(unsigned int cpu) · line 665

Quoted 10 lines

kernel/softirq.c +5 -1

 		 */
 		__do_softirq();
-		rcu_note_context_switch(cpu);
 		local_irq_enable();
 		cond_resched();
+
+		preempt_disable();
+		rcu_note_context_switch(cpu);
+		preempt_enable();
+

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

Quoted 8 lines

Quoted 1 line

	__preempt_count_sub(PREEMPT_ACTIVE);

I agree that if should_resched() returns true as assumed above, then there is no point to invoking rcu_note_context_switch(). However, the case that this code applies to is when should_resched() returns false, but RCU is waiting for a quiescent state from the current CPU. In that case, cond_resched() won't do anything for RCU, and we do need the rcu_note_context_switch().

I've been curious about this for ages, so now is a great time to bite the bullet and ask TheMan. A context switch is not far away, why do we need that quiescent state badly enough to tell what looks like a little white lie to get it immediately?

(I commented it out in an -rt kernel I was testing yesterday, beat it enthusiastically for a while, and box didn't _seem_ to notice that it was missing anything)

Quoted 5 lines · Mike Galbraith

I've been curious about this for ages, so now is a great time to bite the bullet and ask TheMan. A context switch is not far away, why do we need that quiescent state badly enough to tell what looks like a little white lie to get it immediately?

Quoted 3 lines

(I commented it out in an -rt kernel I was testing yesterday, beat it enthusiastically for a while, and box didn't _seem_ to notice that it was missing anything)

Yeah, you do have to have a fairly violent network-based DoS attack to see the difference. Robert Olsson was the first to make this happen back in the day.

Quoted 10 lines · Thomas Gleixner

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

Quoted 1 line

	__preempt_count_sub(PREEMPT_ACTIVE);

I agree that if should_resched() returns true as assumed above, then there is no point to invoking rcu_note_context_switch(). However, the case that this code applies to is when should_resched() returns false, but RCU is waiting for a quiescent state from the current CPU. In that case, cond_resched() won't do anything for RCU, and we do need the rcu_note_context_switch().

Of course, it would be better to avoid the extra RCU work in the common case where cond_resched() does inovke the scheduler. And that is the point of the following patch, which uses cond_resched_rcu_qs(). However, this use of cond_resched_rcu_qs() doesn't work in older kernels. So Calvin's patch is for backporting, and the follow-up patch for future kernels.

Quoted 11 lines · Paul E. McKenney

Quoted 10 lines · Thomas Gleixner

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

Quoted 8 lines

Quoted 1 line

	__preempt_count_sub(PREEMPT_ACTIVE);

I agree that if should_resched() returns true as assumed above, then there is no point to invoking rcu_note_context_switch(). However, the case that this code applies to is when should_resched() returns false, but RCU is waiting for a quiescent state from the current CPU. In that case, cond_resched() won't do anything for RCU, and we do need the rcu_note_context_switch().

So this should be:

   if (!cond_resched()) {

      rcu_note_context_switch();

Hmm?

Quoted 6 lines

Of course, it would be better to avoid the extra RCU work in the common case where cond_resched() does inovke the scheduler. And that is the point of the following patch, which uses cond_resched_rcu_qs(). However, this use of cond_resched_rcu_qs() doesn't work in older kernels. So Calvin's patch is for backporting, and the follow-up patch for future kernels.

I see.

Quoted 21 lines · Thomas Gleixner

Quoted 11 lines · Paul E. McKenney

Quoted 10 lines · Thomas Gleixner

The whole rcu_note_context_switch() in run_ksoftirqd() is silly.

    cond_resched()
	__preempt_count_add(PREEMPT_ACTIVE);

	__schedule();
	     preempt_disable();
	     rcu_note_context_switch();
	     ....

Quoted 8 lines

Quoted 1 line

	__preempt_count_sub(PREEMPT_ACTIVE);

I agree that if should_resched() returns true as assumed above, then there is no point to invoking rcu_note_context_switch(). However, the case that this code applies to is when should_resched() returns false, but RCU is waiting for a quiescent state from the current CPU. In that case, cond_resched() won't do anything for RCU, and we do need the rcu_note_context_switch().

Quoted 3 lines

So this should be:

   if (!cond_resched()) {

	preempt_disable();

Quoted 1 line

      rcu_note_context_switch();

	preempt_enable();
     }

Quoted 2 lines

Hmm?

Going forward, yes, and cond_resched_rcu_qs() in fact does something very similar. For backporting, which is what this patch is for, we are preserving the same double-quiescent-state behavior that existed earlier, meaning minimal perturbation of old releases.

Seem reasonable, or do you really feel strongly about pushing this optimization into -stable?