Re: [PATCH RFC -rt] updated synchronize_all_irqs implementation

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Sep 26, 2007 at 09:16:55AM -0400, Dmitry Torokhov wrote:
> Hi Paul,
> 
> On 9/26/07, Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > On Wed, Sep 26, 2007 at 10:28:33AM +0200, Peter Zijlstra wrote:
> > > On Tue, 25 Sep 2007 18:11:39 -0700 "Paul E. McKenney"
> > > <paulmck@linux.vnet.ibm.com> wrote:
> > >
> > > > On Wed, Sep 26, 2007 at 01:24:47AM +0200, Peter Zijlstra wrote:
> > > > > On Tue, 25 Sep 2007 16:02:45 -0400 (EDT) Steven Rostedt
> > > > > <rostedt@goodmis.org> wrote:
> > > > >
> > > > > > > This would of course require that synchronize_all_irqs() be in the
> > > > > > > RCU code rather than the irq code so that it could access the static
> > > > > > > wakeme_after_rcu() definition and the rcu_synchronize structure.
> > > > > > >
> > > > > > > Thoughts?
> > > > > >
> > > > > > I do like this better. Anyone else care to comment?
> > > > >
> > > > > I'm still wondering why the IRQ users cannot user proper RCU as it
> > > > > stands:
> > > >
> > > > Well, that was my initial proposal.  ;-)
> > >
> > > handler:
> > > > >   rcu_read_lock();
> > > > >   foo = rcu_dereference(bar);
> > > > >   if (foo)
> > > > >     foo();
> > > > >   rcu_read_unlock();
> > > >
> > >
> > > control routine (!handler)
> > > > > vs
> > > > >
> > > > >   rcu_assign(foo, NULL);
> > > > >   synchronize_rcu();
> >
> > Ah, OK -- yes, that was what I originally proposed -- that individual
> > handlers using RCU place the rcu_read_lock() and rcu_read_unlock() as
> > needed.
> >
> > > > > The implicit rcu_read_lock() as placed in handle_IRQ_event() seems
> > > > > misplaced.
> > > >
> > > > OK -- where would you put them instead?  I have them covering the
> > > > call to the handler, so what am I missing here?
> > >
> > > in do_hardirq() (-rt) that is specific to threaded interrupts.
> >
> > My concern there is that some of the functions called from do_hardirq()
> > can loop processing multiple interrupts.  An interrupt storm, otherwise
> > harmless in -rt, could cause a very long RCU read-side critical section
> > if it happened within thread_edge_irq().
> >
> > > That said, I'm wondering if we need this whole extra sync_all_irqs()
> > > thing. I'm just not getting why IRQ handlers should be an implicit RCU
> > > safe context.
> >
> > Because they are in non-rt -- synchronize_sched() is guaranteed to
> > wait for all interrupt handlers.  In contrast, in -rt, synchronize_sched()
> > only waits for hardirq.  So Dmitry Torokhov asked for a primitive
> > that would wait for all irq handlers, whether threaded or not.
> 
> That is correct. IIRC synchronize_sched() was introduced to show that
> it is not related (other than implementation-wise) to RCU mechanisms.

Yep, it was split out a few years back.

> > But given that he has not responded to this thread, perhaps he
> > found that synchronize_irq() worked for him.
> 
> Sorry, I am just being slow.

No problem!

> No, I don't think synchronize_irq() will work for me. While in i8042 I
> know there are 2 possible IRQs (so I'd need 2 calls to
> synchronize_irq()) other drivers may not know what IRQ triggered their
> handler (or whether it was an IRQ at all).
> 
> Actually, I need clarifucation on what you mean by "interrupt
> handlers" in sync_all_irqs(). Right now (if I understand it correctly)
> synchronize_sched() in mainline will wait for completion of all
> IRQ-like contexts. By IRQ-like context I mean code guardede by
> spinlock + IRQ off. Serio (input) drivers have their "interrupt"
> routines run in that IRQ-like context. They may be invoked as a result
> of real IRQ being raised but they also be invoked as a result of
> userspace action of some sort. It all depends on implementation of
> underlying serio port. So if sync_all_irqs() only waits for real IRQ
> handlers to complete it is not sufficient in my case...

The synchronize_all_irqs() will not return until:

1.	All pre-existing hardirqs have completed.

2.	All pre-existing threaded irqs have completed.

3.	All preempt_disable() regions of code have completed.

4.	All irq-disable regions of code have completed.

It will not necessarily wait for all softirqs to complete, but
then again, synchronize_sched() in non-rt might not wait for all
softirqs either, for example, if ksoftirqd is handling softirqs.

Does that do what you need, or am I missing a case that needs
to be covered?

						Thanx, Paul