Re: [ANNOUNCE] 3.2.9-rt17

[Steven Rostedt]

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.

-- Steve


--
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/