Re: [PATCH] kprobes: Fix to delay the kprobes jump optimization

[Paul E. McKenney]

On Fri, Feb 19, 2021 at 10:33:36AM -0800, Paul E. McKenney wrote:
> On Fri, Feb 19, 2021 at 10:18:11AM -0800, Paul E. McKenney wrote:
> > On Fri, Feb 19, 2021 at 12:27:51PM +0100, Uladzislau Rezki wrote:
> > > On Fri, Feb 19, 2021 at 12:23:57PM +0100, Uladzislau Rezki wrote:
> > > > On Fri, Feb 19, 2021 at 12:17:38PM +0100, Sebastian Andrzej Siewior wrote:
> > > > > On 2021-02-19 12:13:01 [+0100], Uladzislau Rezki wrote:
> > > > > > I or Paul will ask for a test once it is settled down :) Looks like
> > > > > > it is, so we should fix for v5.12.
> > > > > 
> > > > > Okay. Since Paul asked for powerpc test on v5.11-rc I wanted check if
> > > > > parts of it are also -stable material.
> > 
> > If Masami's patch works for the PowerPC guys on v5.10-rc7, then it can
> > be backported.  The patch making RCU Tasks initialize itself early won't
> > have any effect and can be left or reverted, as we choose.  The self-test
> > patch will need to be either adjusted or reverted.
> > 
> > However...
> > 
> > The root cause of this problem is that softirq only kind-of works
> > during a window of time during boot.  It works only if the number and
> > duration of softirq handlers during this time is small enough, for some
> > ill-defined notion of "small enough".  If there are too many, whatever
> > that means exactly, then we get failed attempt to awaken ksoftirqd, which
> > (sometimes!) results in a silent hang.  Which, as you pointed out earlier,
> > is a really obnoxious error message.  And any minor change could kick
> > us into silent-hang state because of the heuristics used to hand off
> > to ksoftirqd.  The straw that broke the camel's back and all that.
> > 
> > One approach would be to add WARN_ON_ONCE() so that if softirq tries
> > to awaken ksoftirqd before it is spawned, we get a nice obvious splat.
> > Unfortunately, this gives false positives because there is code that
> > needs a softirq handler to run eventually, but is OK with that handler
> > being delayed until some random point in the early_initcall() sequence.
> > 
> > Besides which, if we are going to add a check, why not use that check
> > just make things work by forcing handler execution to remain within the
> > softirq back-of-interrupt context instead of awakening a not-yet-spawned
> > ksoftirqd?  We can further prevent entry into dyntick-idle state until
> > the ksoftirqd kthreads have been spawned, which means that if softirq
> > handlers must be deferred, they will be resumed within one jiffy by the
> > next scheduler-clock interrupt.
> > 
> > Yes, this can allow softirq handlers to impose large latencies, but only
> > during early boot, long before any latency-sensitive applications can
> > possibly have been created.  So this does not seem like a real problem.
> > 
> > Am I missing something here?
> 
> For definiteness, here is the first part of the change, posted earlier.
> The commit log needs to be updated.  I will post the change that keeps
> the tick going as a reply to this email.

And here it is.

							Thanx, Paul

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

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 9c0ee82..1d4f5b8 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1320,6 +1320,11 @@ static void rcu_prepare_kthreads(int cpu)
  */
 int rcu_needs_cpu(u64 basemono, u64 *nextevt)
 {
+	/* Through early_initcall(), need tick for softirq handlers. */
+	if (!this_cpu_ksoftirqd()) {
+		*nextevt = 1;
+		return 1;
+	}
 	*nextevt = KTIME_MAX;
 	return !rcu_segcblist_empty(&this_cpu_ptr(&rcu_data)->cblist) &&
 		!rcu_rdp_is_offloaded(this_cpu_ptr(&rcu_data));
@@ -1415,6 +1420,12 @@ int rcu_needs_cpu(u64 basemono, u64 *nextevt)
 
 	lockdep_assert_irqs_disabled();
 
+	/* Through early_initcall(), need tick for softirq handlers. */
+	if (!this_cpu_ksoftirqd()) {
+		*nextevt = 1;
+		return 1;
+	}
+
 	/* If no non-offloaded callbacks, RCU doesn't need the CPU. */
 	if (rcu_segcblist_empty(&rdp->cblist) ||
 	    rcu_rdp_is_offloaded(rdp)) {