[PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Nicholas Mc Guire]

>From 46393dc3185026c8500c2b734747d7c8785f3dc9 Mon Sep 17 00:00:00 2001
From: Nicholas Mc Guire <der.herr@hofr.at>
Date: Tue, 19 Nov 2013 23:31:05 -0500
Subject: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

 read_lock_bh/read_unlock_bh unconditionally calls local_bh_disable/enable 
 which already does a migrate_disable/enable - no need for this recursive call.

 patch is on top of 3.12-rt2

 No change of functionality

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 include/linux/rwlock_rt.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
index 853ee36..87f5a1d 100644
--- a/include/linux/rwlock_rt.h
+++ b/include/linux/rwlock_rt.h
@@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
 #define read_lock_bh(lock)				\
 	do {						\
 		local_bh_disable();			\
-		migrate_disable();			\
 		rt_read_lock(lock);			\
 	} while (0)
 
@@ -83,7 +82,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
 #define read_unlock_bh(lock)				\
 	do {						\
 		rt_read_unlock(lock);			\
-		migrate_enable();			\
 		local_bh_enable();			\
 	} while (0)
 
-- 
1.7.2.5

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Peter Zijlstra]

On Wed, Nov 20, 2013 at 11:21:07AM +0100, Nicholas Mc Guire wrote:
> From 46393dc3185026c8500c2b734747d7c8785f3dc9 Mon Sep 17 00:00:00 2001
> From: Nicholas Mc Guire <der.herr@hofr.at>
> Date: Tue, 19 Nov 2013 23:31:05 -0500
> Subject: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh
> 
>  read_lock_bh/read_unlock_bh unconditionally calls local_bh_disable/enable 
>  which already does a migrate_disable/enable - no need for this recursive call.
> 
>  patch is on top of 3.12-rt2
> 
>  No change of functionality
> 
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
>  include/linux/rwlock_rt.h |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
> index 853ee36..87f5a1d 100644
> --- a/include/linux/rwlock_rt.h
> +++ b/include/linux/rwlock_rt.h
> @@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
>  #define read_lock_bh(lock)				\
>  	do {						\
>  		local_bh_disable();			\
> -		migrate_disable();			\
>  		rt_read_lock(lock);			\
>  	} while (0)
>  
> @@ -83,7 +82,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
>  #define read_unlock_bh(lock)				\
>  	do {						\
>  		rt_read_unlock(lock);			\
> -		migrate_enable();			\
>  		local_bh_enable();			\
>  	} while (0)

So the problem with this patch and the next is that:

	read_lock_bh();

	read_unlock();
	local_bh_enable();

Is a valid pattern; and you'll notice that the release part has 2
migrate put refs. So if you can make a patch similar to:

lkml.kernel.org/r/20131120162736.624493595@infradead.org

That allows read_lock_bh() to obtain 2 migrate disable refs in one go,
then it would all work out just fine.

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Nicholas Mc Guire]

On Thu, 21 Nov 2013, Peter Zijlstra wrote:

> On Wed, Nov 20, 2013 at 11:21:07AM +0100, Nicholas Mc Guire wrote:
> > From 46393dc3185026c8500c2b734747d7c8785f3dc9 Mon Sep 17 00:00:00 2001
> > From: Nicholas Mc Guire <der.herr@hofr.at>
> > Date: Tue, 19 Nov 2013 23:31:05 -0500
> > Subject: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh
> > 
> >  read_lock_bh/read_unlock_bh unconditionally calls local_bh_disable/enable 
> >  which already does a migrate_disable/enable - no need for this recursive call.
> > 
> >  patch is on top of 3.12-rt2
> > 
> >  No change of functionality
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >  include/linux/rwlock_rt.h |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
> > index 853ee36..87f5a1d 100644
> > --- a/include/linux/rwlock_rt.h
> > +++ b/include/linux/rwlock_rt.h
> > @@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> >  #define read_lock_bh(lock)				\
> >  	do {						\
> >  		local_bh_disable();			\
> > -		migrate_disable();			\
> >  		rt_read_lock(lock);			\
> >  	} while (0)
> >  
> > @@ -83,7 +82,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> >  #define read_unlock_bh(lock)				\
> >  	do {						\
> >  		rt_read_unlock(lock);			\
> > -		migrate_enable();			\
> >  		local_bh_enable();			\
> >  	} while (0)
> 
> So the problem with this patch and the next is that:
> 
> 	read_lock_bh();
> 
> 	read_unlock();
> 	local_bh_enable();
> 
> Is a valid pattern; and you'll notice that the release part has 2
> migrate put refs. So if you can make a patch similar to:
> 
> lkml.kernel.org/r/20131120162736.624493595@infradead.org
> 
> That allows read_lock_bh() to obtain 2 migrate disable refs in one go,
> then it would all work out just fine.

well that was what I ment with the nned to understand the interleavings of
better - playing with a cocci patch to catch these "equivalent" call sequences
unfortunately it did not trigger - so I promply concluded that all is 
fine...

thx!
hofrat

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Nicholas Mc Guire]

On Thu, 21 Nov 2013, Peter Zijlstra wrote:

> On Wed, Nov 20, 2013 at 11:21:07AM +0100, Nicholas Mc Guire wrote:
> > From 46393dc3185026c8500c2b734747d7c8785f3dc9 Mon Sep 17 00:00:00 2001
> > From: Nicholas Mc Guire <der.herr@hofr.at>
> > Date: Tue, 19 Nov 2013 23:31:05 -0500
> > Subject: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh
> > 
> >  read_lock_bh/read_unlock_bh unconditionally calls local_bh_disable/enable 
> >  which already does a migrate_disable/enable - no need for this recursive call.
> > 
> >  patch is on top of 3.12-rt2
> > 
> >  No change of functionality
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >  include/linux/rwlock_rt.h |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
> > index 853ee36..87f5a1d 100644
> > --- a/include/linux/rwlock_rt.h
> > +++ b/include/linux/rwlock_rt.h
> > @@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> >  #define read_lock_bh(lock)				\
> >  	do {						\
> >  		local_bh_disable();			\
> > -		migrate_disable();			\
> >  		rt_read_lock(lock);			\
> >  	} while (0)
> >  
> > @@ -83,7 +82,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> >  #define read_unlock_bh(lock)				\
> >  	do {						\
> >  		rt_read_unlock(lock);			\
> > -		migrate_enable();			\
> >  		local_bh_enable();			\
> >  	} while (0)
> 
> So the problem with this patch and the next is that:
> 
> 	read_lock_bh();
> 
> 	read_unlock();
> 	local_bh_enable();
> 
> Is a valid pattern; and you'll notice that the release part has 2
> migrate put refs. So if you can make a patch similar to:
> 
> lkml.kernel.org/r/20131120162736.624493595@infradead.org
> 
> That allows read_lock_bh() to obtain 2 migrate disable refs in one go,
> then it would all work out just fine.

yup thats what was causing the oops - fixed by the patch below
with this my two boxes here survived cyclictest over night
showing the same results as without the recursive calls to
migrate_disable/enable removed.


>From 2c8e669b691b825c0ed2a02bd7a698d8ed5c6d29 Mon Sep 17 00:00:00 2001
From: Nicholas Mc Guire <der.herr@hofr.at>
Date: Thu, 21 Nov 2013 18:22:55 -0500
Subject: [PATCH] rebalance locks by converting write_lock_bh to write_lock+local_bh_disable
 
 This patch just rebalances the lock api

 in __neigh_event_send write_lock_bh(&neigh->lock) is implicitly balanced by
 write_unlock(&neigh->lock)+local_bh_disable() - while this is equivalent with
 respect to the effective low level locking primitives it breaks balancing
 in the locking api. This makes automatic lock-checking trigger false 
 positives, creates an implicit dependency between *_lock_bh and *_lock 
 functions as well as making the extremly simply locking of net core even
 easier to understand.

 No change of functionality

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 net/core/neighbour.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index ca15f32..d681c75 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -966,7 +966,8 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
 	int rc;
 	bool immediate_probe = false;
 
-	write_lock_bh(&neigh->lock);
+	local_bh_disable();
+	write_lock(&neigh->lock);
 
 	rc = 0;
 	if (neigh->nud_state & (NUD_CONNECTED | NUD_DELAY | NUD_PROBE))
-- 
1.7.2.5

[PATCH] condition migration_disable on lock acquisition

[Nicholas Mc Guire]

>From a73aed45d686f800871ba0342cd223bf23e70302 Mon Sep 17 00:00:00 2001
From: Nicholas Mc Guire <der.herr@hofr.at>
Date: Thu, 21 Nov 2013 22:52:30 -0500
Subject: [PATCH] condition migration_disable on lock acquisition

 No need to unconditionally migrate_disable (what is it protecting ?) and
 re-enable on failure to acquire the lock.
 This patch moves the migrate_disable to be conditioned on sucessful lock
 acquisition only.

 No change of functionality

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 kernel/rt.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/rt.c b/kernel/rt.c
index 433ae42..4b2c4a9 100644
--- a/kernel/rt.c
+++ b/kernel/rt.c
@@ -182,11 +182,10 @@ int __lockfunc rt_write_trylock(rwlock_t *rwlock)
 {
 	int ret = rt_mutex_trylock(&rwlock->lock);
 
-	migrate_disable();
-	if (ret)
+	if (ret) {
 		rwlock_acquire(&rwlock->dep_map, 0, 1, _RET_IP_);
-	else
-		migrate_enable();
+		migrate_disable();
+	}
 
 	return ret;
 }
-- 
1.7.2.5

[PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave

[Nicholas Mc Guire]

>From f13384c888660f34d5f888ca96e55ad46c3a27b6 Mon Sep 17 00:00:00 2001
From: Nicholas Mc Guire <der.herr@hofr.at>
Date: Thu, 21 Nov 2013 23:04:50 -0500
Subject: [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave

 rt_write_trylock_irqsave unconditionally calls rt_write_trylock which will
 disable migration if the lock was sucessfully acquired.
 This patch drops the recursive migrate_disable/enable in
 rt_write_trylock_irqsave and rt_write_unlock_irq respecttively

 No change of functionality

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 include/linux/rwlock_rt.h |    1 -
 kernel/rt.c               |    4 +---
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
index 9a5fe26..f6c7612 100644
--- a/include/linux/rwlock_rt.h
+++ b/include/linux/rwlock_rt.h
@@ -105,7 +105,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
 		typecheck(unsigned long, flags);	\
 		(void) flags;				\
 		rt_read_unlock(lock);			\
-		migrate_enable();			\
 	} while (0)
 
 #define write_unlock_irqrestore(lock, flags) \
diff --git a/kernel/rt.c b/kernel/rt.c
index 4b2c4a9..71d26a4 100644
--- a/kernel/rt.c
+++ b/kernel/rt.c
@@ -196,10 +196,8 @@ int __lockfunc rt_write_trylock_irqsave(rwlock_t *rwlock, unsigned long *flags)
 	int ret;
 
 	*flags = 0;
-	migrate_disable();
 	ret = rt_write_trylock(rwlock);
-	if (!ret)
-		migrate_enable();
+
 	return ret;
 }
 EXPORT_SYMBOL(rt_write_trylock_irqsave);
-- 
1.7.2.5

[PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave - the right one this time...

[Nicholas Mc Guire]

sorry - sent out the wrong file before

thx!
hofrat

>From f7702e3f2077b29b51172662fb988ce7fec9a985 Mon Sep 17 00:00:00 2001
From: Nicholas Mc Guire <der.herr@hofr.at>
Date: Fri, 22 Nov 2013 00:04:51 -0500
Subject: [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave

 rt_write_trylock_irqsave unconditionally calls rt_write_trylock which will     
 disable migration if the lock was sucessfully acquired.                        
 This patch drops the recursive migrate_disable/enable in                       
 rt_write_trylock_irqsave and rt_write_unlock_irq respecttively                 

 patch is on top of 3.12-rt2
                                                                                
 No change of functionality

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 include/linux/rwlock_rt.h |    1 -
 kernel/rt.c               |    4 +---
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
index 9a5fe26..2dbabe2 100644
--- a/include/linux/rwlock_rt.h
+++ b/include/linux/rwlock_rt.h
@@ -113,7 +113,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
 		typecheck(unsigned long, flags);	\
 		(void) flags;				\
 		rt_write_unlock(lock);			\
-		migrate_enable();			\
 	} while (0)
 
 #endif
diff --git a/kernel/rt.c b/kernel/rt.c
index 4b2c4a9..71d26a4 100644
--- a/kernel/rt.c
+++ b/kernel/rt.c
@@ -196,10 +196,8 @@ int __lockfunc rt_write_trylock_irqsave(rwlock_t *rwlock, unsigned long *flags)
 	int ret;
 
 	*flags = 0;
-	migrate_disable();
 	ret = rt_write_trylock(rwlock);
-	if (!ret)
-		migrate_enable();
+
 	return ret;
 }
 EXPORT_SYMBOL(rt_write_trylock_irqsave);
-- 
1.7.2.5

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2013-11-20 11:21:07 [+0100]:

>diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
>index 853ee36..87f5a1d 100644
>--- a/include/linux/rwlock_rt.h
>+++ b/include/linux/rwlock_rt.h
>@@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> #define read_lock_bh(lock)				\
> 	do {						\
> 		local_bh_disable();			\
>-		migrate_disable();			\
> 		rt_read_lock(lock);			\
> 	} while (0)
> 

local_bh_disable() does only.
| add_preempt_count()

Where is that second migrate_disable() comming from?

Sebastian

Re: [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2013-11-22 05:44:39 [+0100]:

> rt_write_trylock_irqsave unconditionally calls rt_write_trylock which will
> disable migration if the lock was sucessfully acquired.
> This patch drops the recursive migrate_disable/enable in
> rt_write_trylock_irqsave and rt_write_unlock_irq respecttively
>
> No change of functionality

I think you change functionality because you remove code. If you would
cut a big function into two small functions then I would agree that the
functionality has not been changed. But…

>index 9a5fe26..f6c7612 100644
>--- a/include/linux/rwlock_rt.h
>+++ b/include/linux/rwlock_rt.h
>@@ -105,7 +105,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> 		typecheck(unsigned long, flags);	\
> 		(void) flags;				\
> 		rt_read_unlock(lock);			\
>-		migrate_enable();			\
> 	} while (0)

You are removing that from read_unlock_irqrestore() instead of
write_unlock_irqrestore() which comes next:

> 
> #define write_unlock_irqrestore(lock, flags) \
>diff --git a/kernel/rt.c b/kernel/rt.c
>index 4b2c4a9..71d26a4 100644
>--- a/kernel/rt.c
>+++ b/kernel/rt.c
>@@ -196,10 +196,8 @@ int __lockfunc rt_write_trylock_irqsave(rwlock_t *rwlock, unsigned long *flags)
> 	int ret;
> 
> 	*flags = 0;
>-	migrate_disable();
> 	ret = rt_write_trylock(rwlock);
>-	if (!ret)
>-		migrate_enable();
>+

The amount of users of rt_write_trylock_irqsave() is so small that
nobody noticed this bug:

 write_trylock_irqsave()
  ->rt_write_trylock_irqsave()
  -> m_d()
    -> rt_write_trylock()
    -> m_d()

That means we called migrate_disable() twice. Now comes the unlock path:
 write_unlock_irqrestore()
  -> rt_write_unlock()
  -> m_e()

That means migrations still needs to be enabled.

I would however prefer your optimisation where you remove the pointless
m_e() in rt_write_trylock_irqsave();

> 	return ret;
> }
> EXPORT_SYMBOL(rt_write_trylock_irqsave);

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave - the right one this time...

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2013-11-22 06:09:34 [+0100]:

>
>sorry - sent out the wrong file before

Please see me previous email on that one with the wrong file :)

>thx!
>hofrat

Sebastian

Re: [PATCH] condition migration_disable on lock acquisition

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2013-11-22 05:42:23 [+0100]:

> No need to unconditionally migrate_disable (what is it protecting ?) and
> re-enable on failure to acquire the lock.
> This patch moves the migrate_disable to be conditioned on sucessful lock
> acquisition only.
>
> No change of functionality
applied with out this line^

>Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>

Sebastian

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Nicholas Mc Guire]

On Fri, 22 Nov 2013, Sebastian Andrzej Siewior wrote:

> * Nicholas Mc Guire | 2013-11-20 11:21:07 [+0100]:
> 
> >diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h
> >index 853ee36..87f5a1d 100644
> >--- a/include/linux/rwlock_rt.h
> >+++ b/include/linux/rwlock_rt.h
> >@@ -53,7 +53,6 @@ extern void __rt_rwlock_init(rwlock_t *rwlock, char *name, struct lock_class_key
> > #define read_lock_bh(lock)				\
> > 	do {						\
> > 		local_bh_disable();			\
> >-		migrate_disable();			\
> > 		rt_read_lock(lock);			\
> > 	} while (0)
> > 
> 
> local_bh_disable() does only.
> | add_preempt_count()
> 
> Where is that second migrate_disable() comming from?
>
hmmm. am I looking at the wrong code path ?

kernel/softirq.c
void local_bh_disable(void)
{
        migrate_disable();
        current->softirq_nestcnt++;
}

confused... will check again.
hofrat 

Re: [PATCH] condition migration_disable on lock acquisition

[Thomas Gleixner]

On Fri, 22 Nov 2013, Nicholas Mc Guire wrote:

> >From a73aed45d686f800871ba0342cd223bf23e70302 Mon Sep 17 00:00:00 2001
> From: Nicholas Mc Guire <der.herr@hofr.at>
> Date: Thu, 21 Nov 2013 22:52:30 -0500
> Subject: [PATCH] condition migration_disable on lock acquisition
> 
>  No need to unconditionally migrate_disable (what is it protecting ?) and
>  re-enable on failure to acquire the lock.
>  This patch moves the migrate_disable to be conditioned on sucessful lock
>  acquisition only.
> 
>  No change of functionality

Well it is a change of functionality, but it has no functional impact.

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2013-11-23 00:39:25 [+0100]:

>hmmm. am I looking at the wrong code path ?
No, it was me, sorry for the confusion.
So this patch should work. What I am concerned is what Peter mentioned.
It seems that we have no user of this "alternative" unlock path right
now but it could become a ticking time bomb.

>hofrat 

Sebastian

Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh

[Nicholas Mc Guire]

On Fri, 29 Nov 2013, Sebastian Andrzej Siewior wrote:

> * Nicholas Mc Guire | 2013-11-23 00:39:25 [+0100]:
> 
> >hmmm. am I looking at the wrong code path ?
> No, it was me, sorry for the confusion.
> So this patch should work. What I am concerned is what Peter mentioned.
> It seems that we have no user of this "alternative" unlock path right
> now but it could become a ticking time bomb.
>
yes that is a concern. I scaned the kernel source tree with some cocci scripts 
to find these problems. And actually I only found 3 (1 with write_lock_bh, 2 
with spin_lock_bh ), all 3 in net/core/*, but the maintainer does not seem 
willing to include these cleanups so those cleanups do not seem doable.

the found locations are:
file:linux-stable-rt4/net/core/sock.c lock split at lines 2382 2392 2397
file:linux-stable-rt4/net/ipv4/inet_hashtables.c lock split at lines 571 577 581
file:linux-stable-rt4/net/core/neighbour.c lock split at lines 969 1024 1025

The basic model of the scanner is like below, might be useful for other 
problems as well. you need a whole set of the possible variations and 
then simply brute force iterate over the kernel with 
 find . -name "*.c" -exec spatch -sp_file read_lock.cocci {} \; 

@r1@
identifier f;
expression E;
position p1,p2,p3;
@@
f(...) {
<...
(
read_lock_bh(E)@p1;
...
read_unlock(E)@p2;
...
local_bh_enable()@p3;
|
local_bh_disable()@p1;
...
read_lock(E)@p2;
...
read_unlock_bh(E)@p3;
)
...>
}

@script:python@
p1 << r1.p1;
p2 << r1.p2;
p3 << r1.p3;
@@
print "file:%s lock split at lines %s %s %s" % (p1[0].file,p1[0].line, p2[0].line, p3[0].line)

so there are ways to defuse these timebombs 
the problem is more the reluctance of net/core to accept such patches
as David Miller considers the lock splitting to be a valid locking idiom
so its probably not woth pushing this further.

thx!
hofrat