On Tue, 27 Feb 2024 20:42:24 -0800 Paul E. McKenney wrote:

On Tue, Feb 27, 2024 at 07:10:01PM -0800, Jakub Kicinski wrote:

quoted

On Tue, 27 Feb 2024 10:32:22 -0800 Paul E. McKenney wrote:  

quoted

The theory is that PREEMPT_RCU kernels have preemption, and get their
quiescent states that way.  

But that doesn't work well enough?

Assuming that's the case why don't we add it with the inverse ifdef
condition next to the cond_resched() which follows a few lines down?

			skb_defer_free_flush(sd);
+
+			if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+				rcu_softirq_qs();
+
			local_bh_enable();

			if (!repoll)
				break;

			cond_resched();
		}

We won't repoll majority of the time.  

I am not completely clear on what you are proposing, but one complication
is that We need preemption disabled across calls to rcu_softirq_qs()
and we cannot have preemption disabled across calls to cond_resched().

I was thinking of using rcu_all_qs(), like cond_resched() does.
Not sure how it compares in terms of functionality and cost.

Another complication is that although CONFIG_PREEMPT_RT kernels are
built with CONFIG_PREEMPT_RCU, the reverse is not always the case.
And if we are not repolling, don't we have a high probability of doing
a voluntary context when we reach napi_thread_wait() at the beginning
of that loop?

Very much so, which is why adding the cost of rcu_softirq_qs()
for every NAPI run feels like an overkill.