Subject: Re: [PATCH] rebalance locks by converting write_lock_bh towrite_lock+local_bh_disable

From: Nicholas Mc Guire <[email-masked]>
Date: Mon, 25 Nov 2013 11:37:51 +0100

On Mon, 25 Nov 2013, David Laight wrote:

> > in one of the cases I do not understand the intent behind the split:
> > in net/core/sock.c:lock_sock_fast
> > 
> > 	spin_lock_bh(&sk->sk_lock.slock);
> > 	...
> >         spin_unlock(&sk->sk_lock.slock);
> >         /*
> >          * The sk_lock has mutex_lock() semantics here:
> >          */
> >         mutex_acquire(&sk->sk_lock.dep_map, 0, 0, _RET_IP_);
> >         local_bh_enable();
> > 
> >  I think that
> > 
> > 	spin_lock_bh(&sk->sk_lock.slock);
> > 	...
> >         /*
> >          * The sk_lock has mutex_lock() semantics here:
> >          */
> >         mutex_acquire(&sk->sk_lock.dep_map, 0, 0, _RET_IP_);
> >         spin_unlock_bh(&sk->sk_lock.slock);
> > 
> >  should be equivalent ?
> 
> You've added a lock ordering that wasn't there before.
> Also I suspect that mutex_acquire() might be allowed to sleep,
> whereas you shouldn't sleep with a spin lock held.
>
mutex_acquire is not a lock but a lockdep entry. 
As far as I understand it it nither can sleep nor is there a change of 
order here. Am I missing something ?

thx!
hofrat