All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Mc Guire <der.herr@hofr.at>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-rt-users@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Andreas Platschek <platschek@ict.tuwien.ac.at>
Subject: Re: [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh
Date: Sat, 30 Nov 2013 02:58:55 +0100	[thread overview]
Message-ID: <20131130015855.GA8114@opentech.at> (raw)
In-Reply-To: <20131129145443.GC31099@linutronix.de>

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

      reply	other threads:[~2013-11-30  1:58 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-20 10:21 [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh Nicholas Mc Guire
2013-11-21 10:22 ` Peter Zijlstra
2013-11-21 11:14   ` Nicholas Mc Guire
2013-11-21 23:58   ` Nicholas Mc Guire
2013-11-22  4:42 ` [PATCH] condition migration_disable on lock acquisition Nicholas Mc Guire
2013-11-22 18:39   ` Sebastian Andrzej Siewior
2013-11-27  0:26   ` Thomas Gleixner
2013-11-22  4:44 ` [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave Nicholas Mc Guire
2013-11-22 17:15   ` Sebastian Andrzej Siewior
2013-11-22  5:09 ` [PATCH] drop recursive migrate_disable in rt_write_trylock_irqsave - the right one this time Nicholas Mc Guire
2013-11-22 17:17   ` Sebastian Andrzej Siewior
2013-11-22 16:12 ` [PATCH 1/2] remove recursive call to migrate_disable in read_lock_bh Sebastian Andrzej Siewior
2013-11-22 23:39   ` Nicholas Mc Guire
2013-11-29 14:54     ` Sebastian Andrzej Siewior
2013-11-30  1:58       ` Nicholas Mc Guire [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131130015855.GA8114@opentech.at \
    --to=der.herr@hofr.at \
    --cc=bigeasy@linutronix.de \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=platschek@ict.tuwien.ac.at \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.