linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-rt-users <linux-rt-users@vger.kernel.org>
Subject: Re: [ANNOUNCE] 3.2.9-rt17
Date: Fri, 9 Mar 2012 01:20:30 +0100 (CET)	[thread overview]
Message-ID: <alpine.LFD.2.02.1203090111240.2742@ionos> (raw)
In-Reply-To: <1331237441.25686.469.camel@gandalf.stny.rr.com>

On Thu, 8 Mar 2012, Steven Rostedt wrote:

> On Thu, 2012-03-08 at 20:39 +0100, Peter Zijlstra wrote:
> 
> > > For example, we have:
> > > 
> > > @@ -410,7 +411,7 @@ static inline struct dentry *dentry_kill
> > >         if (inode && !spin_trylock(&inode->i_lock)) {
> > >  relock:
> > >                 seq_spin_unlock(&dentry->d_lock);
> > > -               cpu_relax();
> > > +               cpu_chill();
> > >                 return dentry; /* try again with same dentry */
> > >         }
> > > 
> > > By doing the test at the trylock, we can easily hit the deadlock,
> > > because we still hold dentry->d_lock. But by moving the block to the
> > > cpu_chill(), then we are less likely to hit the deadlock.
> > 
> > Actually hitting the deadlock isn't a problem, and doing it in the place
> > of the trylock has the distinct advantage that you can actually get the
> > lock and continue like you want.
> 
> By doing a spin_trydeadlock() while still holding the d_lock, if the
> holder of the i_lock was blocked on that d_lock then it would detect the
> failure, and release the lock and continue the loop. This doesn't solve
> anything. Just because we released the lock, we are still preempting the
> holder of the d_lock, and if we are higher in priority, we will never
> let the owner run.
> 
> That's why I recommended doing it after releasing the lock. Of course
> the d_put() is so screwed up because it's not just two locks involved,
> it's a reverse chain, where this probably wont help.
> 
> But just sleeping a tick sounds like a heuristic that may someday fail.

And it's the only way to deal with it sanely simply because after
dropping d_lock you CANNOT dereference inode->i_lock anymore. Care to
read the fricking locking rules of dcache and friends ?

So yopu need to replace the trylock by try_deadlock() and boost the
lock owner w/o blocking on the lock. That means that you need some
other means to tell the lock owner that you are waiting for it w/o
actually waiting. After you return from try_deadlock() you drop d_lock
and wait for some magic event which is associated to the unlock of
i_lock. There is no other way to deal with reverse lock ordering,
period.

Now go and imagine the mess you have to create to make this work under
all circumstances. My reference to the multiple reader boosting was
not for fun. Though I consider that try_deadlock() idea to be in the
same category of insanity as the original attempts of implementing
requeue_pi with ownerless but boosted rtmutexes ....

Thanks,

	tglx

  parent reply	other threads:[~2012-03-09  0:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-07 21:49 [ANNOUNCE] 3.2.9-rt17 Thomas Gleixner
2012-03-08 18:23 ` Steven Rostedt
2012-03-08 18:28   ` Peter Zijlstra
2012-03-08 18:42     ` Steven Rostedt
2012-03-08 19:39       ` Peter Zijlstra
2012-03-08 20:10         ` Steven Rostedt
2012-03-08 20:26           ` Peter Zijlstra
2012-03-08 21:08             ` Steven Rostedt
2012-03-08 21:20               ` Peter Zijlstra
2012-03-08 21:25                 ` Steven Rostedt
2012-03-08 21:28                   ` Peter Zijlstra
2012-03-08 21:36                     ` Steven Rostedt
2012-03-08 21:37                       ` Peter Zijlstra
2012-03-08 21:44                         ` Steven Rostedt
2012-03-08 21:54                           ` Peter Zijlstra
2012-03-08 22:13                             ` Steven Rostedt
2012-03-08 22:20                               ` Peter Zijlstra
2012-03-08 22:27                                 ` Steven Rostedt
2012-03-09  4:17                                 ` Steven Rostedt
2012-03-09  0:33                   ` Thomas Gleixner
2012-03-09  3:08                     ` Steven Rostedt
2012-03-09  0:20           ` Thomas Gleixner [this message]
2012-03-09  2:50             ` Steven Rostedt
2012-03-09 10:23               ` Thomas Gleixner
2012-03-09 12:51                 ` Steven Rostedt
2012-03-08 19:48       ` Peter Zijlstra
2012-03-08 20:01         ` Steven Rostedt
2012-03-08 20:08           ` Peter Zijlstra

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=alpine.LFD.2.02.1203090111240.2742@ionos \
    --to=tglx@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=peterz@infradead.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).