Re: [PATCH 1/1] softirq: Use a dedicated thread for timer wakeups on PREEMPT_RT.

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2024-10-23 08:30:18 [+0200], To Frederic Weisbecker wrote:
> > > > > +void raise_timer_softirq(void)
> > > > > +{
> > > > > +	unsigned long flags;
> > > > > +
> > > > > +	local_irq_save(flags);
> > > > > +	raise_ktimers_thread(TIMER_SOFTIRQ);
> > > > > +	wake_timersd();
> > > > 
> > > > This is supposed to be called from hardirq only, right?
> > > > Can't irq_exit_rcu() take care of it? Why is it different
> > > > from HRTIMER_SOFTIRQ ?
> > > 
> > > Good question. This shouldn't be any different compared to the hrtimer
> > > case. This is only raised in hardirq, so yes, the irq_save can go away
> > > and the wake call, too.
> > 
> > Cool. You can add lockdep_assert_in_irq() within raise_ktimers_thread() for
> > some well deserved relief :-)
> 
> If you want to, sure. I would add them to both raise functions.

That function (run_local_timers()) was in past also called from other
places like the APIC IRQ but all this is gone now. The reason why I
added the wake and the local_irq_save() is because it uses
raise_softirq() instead raise_softirq_irqoff(). And raise_softirq() was
used since it was separated away from tasklets.

Now, raise_timer_softirq() is a function within softirq.c because it
needs to access task_struct timersd which was only accessible there. It
has been made public later due to the rcutorture bits so it could be
very much be made inline and reduced to just raise_ktimers_thread().
I tend to make TIMER_SOFTIRQ use also raise_softirq_irqoff() to make it
look the same. That lockdep_assert_in_irq() is probably cheap but it
might look odd why RT needs or just TIMER and not HRTIMER.

> > Thanks.

Sebastian