Re: [PATCH RFC -rt] updated synchronize_all_irqs implementation

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

On Wed, Sep 26, 2007 at 10:54:46PM +0200, Peter Zijlstra wrote:
> 
> On Wed, 2007-09-26 at 12:55 -0700, Paul E. McKenney wrote:
> 
> > Well, we could make spin_lock_irqsave() invoke rcu_read_lock() and
> > spin_lock_irqrestore() invoke rcu_read_unlock(), with similar adjustments
> > to the other primitives in this group.  Then RCU priority boosting would
> > kick in if configured.
> 
> Might be me, but 'hiding' the RCU scope like that makes my skin crawl.
> What is wrong with using rcu_read_lock() explicitly where you depend on
> it? It even makes the code cleaner in that it documents the usage.
> 
> These blanket locks like lock_kernel(), preempt_disable(),
> local_irq_disable() etc. are very hard to get rid of because they don't
> document what is protected.
> 
> Please lets not add to this mess and keep all this explicit.
> 
> Yes, -rt changes the preemption model, and that has concequences. But
> IMHO the resulting code is cleaner in most cases.

I am not going to argue with you on this one!!!

But it is not me that needs convincing.

> I'd go as far as to depricate synchronize_sched() and replace all its
> users with proper RCU.

But this might be going too far, though you might well be able to
convince me.  Reducing RCU API proliferation would be a good thing
in general...

> The more I think of it, the more I dislike this synchronize_all_irqs()
> and the 'magic' property of irq handlers.

So I produced a 'magic' patch?  Cool!!!  ;-)

Seriously, I agree that using explicit rcu_read_lock() where needed would
be much better from a readability and robustness viewpoint.  However,
if the goal is to support the existing code unmodified, then this patch
is one solution.  I would be just as happy to see it become unnecessary
as to see it go in, as long as the problem is solved one way or another.

							Thanx, Paul