Re: [PATCH] netpoll: use non-BH variant of RCU

From: Paul E. McKenney
Date: Thu Sep 02 2010 - 13:26:31 EST


On Sat, Aug 14, 2010 at 11:50:33PM -0700, David Miller wrote:
> From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Date: Fri, 13 Aug 2010 13:51:57 -0400
>
> > On Fri, Aug 13, 2010 at 09:29:12AM -0700, Paul E. McKenney wrote:
> >> But all in all, mightn't it be easier to remove the checks from
> >> _local_bh_enable(), and then just use rcu_read_lock_bh()? Have those
> >> checks really been that helpful in finding bugs? ;-)
> >
> > You are right. It would be much simpler to simply have it not
> > warn.
>
> For now I'm going to assume that this is how the issue will be
> addressed.

And here is the patch, at long last. Adding Ingo and Peter Z to CC
in case there is some problem with this approach that I cannot see.

Thanx, Paul

commit 095d050d5d04344d3cdd4ff8558ea8709e3c4beb
Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Date: Thu Sep 2 10:07:21 2010 -0700

softirq: adjust error check

The error check for _local_bh_enable_ip() warns on both in_irq() and
irqs_disabled(). The check for in_irq() is necessary, because a hardirq
might well have interrupted bh execution, in which case it is simply
not possible for the hardirq to exclude the bh that got interrupted.

However, it is perfectly reasonable to disable bh while hardirqs are
disabled, and this in fact promotes common code. This commit therefore
removes the irqs_disabled() check.

Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 07b4f1b..3cff1bb 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -140,7 +140,7 @@ EXPORT_SYMBOL(_local_bh_enable);

static inline void _local_bh_enable_ip(unsigned long ip)
{
- WARN_ON_ONCE(in_irq() || irqs_disabled());
+ WARN_ON_ONCE(in_irq());
#ifdef CONFIG_TRACE_IRQFLAGS
local_irq_disable();
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/