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

[Uladzislau Rezki]

On Mon, Feb 22, 2021 at 10:16:08AM -0800, Paul E. McKenney wrote:
> On Mon, Feb 22, 2021 at 06:16:05PM +0100, Uladzislau Rezki wrote:
> > On Mon, Feb 22, 2021 at 07:09:03AM -0800, Paul E. McKenney wrote:
> > > On Mon, Feb 22, 2021 at 01:54:31PM +0100, Uladzislau Rezki wrote:
> > > > On Mon, Feb 22, 2021 at 11:21:04AM +0100, Sebastian Andrzej Siewior wrote:
> > > > > On 2021-02-19 10:33:36 [-0800], Paul E. McKenney wrote:
> > > > > > 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.
> > > > > …
> > > > > > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > > > > > index 9d71046..ba78e63 100644
> > > > > > --- a/kernel/softirq.c
> > > > > > +++ b/kernel/softirq.c
> > > > > > @@ -209,7 +209,7 @@ static inline void invoke_softirq(void)
> > > > > >  	if (ksoftirqd_running(local_softirq_pending()))
> > > > > >  		return;
> > > > > >  
> > > > > > -	if (!force_irqthreads) {
> > > > > > +	if (!force_irqthreads || !__this_cpu_read(ksoftirqd)) {
> > > > > >  #ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
> > > > > >  		/*
> > > > > >  		 * We can safely execute softirq on the current stack if
> > > > > > @@ -358,8 +358,8 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
> > > > > >  
> > > > > >  	pending = local_softirq_pending();
> > > > > >  	if (pending) {
> > > > > > -		if (time_before(jiffies, end) && !need_resched() &&
> > > > > > -		    --max_restart)
> > > > > > +		if (!__this_cpu_read(ksoftirqd) ||
> > > > > > +		    (time_before(jiffies, end) && !need_resched() && --max_restart))
> > > > > >  			goto restart;
> > > > > 
> > > > > This is hunk shouldn't be needed. The reason for it is probably that the
> > > > > following wakeup_softirqd() would avoid further invoke_softirq()
> > > > > performing the actual softirq work. It would leave early due to
> > > > > ksoftirqd_running(). Unless I'm wrong, any raise_softirq() invocation
> > > > > outside of an interrupt would do the same. 
> > > 
> > > And it does pass the rcutorture test without that hunk:
> > > 
> > > tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 2 --configs "TREE03" --kconfig "CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y" --bootargs "threadirqs=1" --trust-make
> > > 
> > Yep. I have tested that patch also. It works for me as well. So
> > technically i do not see any issues from the first glance but of
> > course it should be reviewed by the softirq people to hear their
> > opinion.
> > 
> > IRQs are enabled, so it can be handled from an IRQ tail until
> > ksoftirqd threads are spawned.
> 
> And if I add "CONFIG_NO_HZ_IDLE=y CONFIG_HZ_PERIODIC=n" it still works,
> even if I revert my changes to rcu_needs_cpu().  Should I rely on this
> working globally?  ;-)
> 
There might be corner cases which we are not aware of so far. From the
other hand what the patch does is simulating the !threadirqs behaviour
during early boot. In that case we know that handling of SW irqs from
real-irq tail works :)

--
Vlad Rezki