Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 Apr 2017 19:56:50 +0800
From:      Yubin Ruan <ablacktshirt@gmail.com>
To:        Chris Torek <torek@elf.torek.net>, imp@bsdimp.com
Cc:        ed@nuxi.nl, freebsd-hackers@freebsd.org, rysto32@gmail.com
Subject:   Re: Understanding the FreeBSD locking mechanism
Message-ID:  <aa5f22f4-0bec-f2a4-554b-f0055398eb7d@gmail.com>
In-Reply-To: <201704120755.v3C7tYUH016699@elf.torek.net>
References:  <201704120755.v3C7tYUH016699@elf.torek.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On 2017年04月12日 15:55, Chris Torek wrote:
>> clear. How can I distinguish these two conditions? I mean, whether I
>> am using my own state/stack or borrowing others' state.
>
> You choose it when you establish your interrupt handler.  If you
> say you are a filter interrupt, then you *are* one, and the rest
> of your code must be written as one.  Unless you know what you
> are doing, don't do this, and then you *aren't* one and the rest
> of your code can be written using the much more relaxed model.
>
>> What do you mean by "must take MTX_SPIN locks 'inside' any outer
>> MTX_DEF locks?
>
> This means that any code path that is going to hold a spin-type
> lock must obtain it while already holding any applicable non-spin
> locks.  For instance, if we look at <sys/proc.h> we find these:
>
> 	#define	PROC_STATLOCK(p)	mtx_lock_spin(&(p)->p_statmtx)
> 	#define	PROC_ITIMLOCK(p)	mtx_lock_spin(&(p)->p_itimmtx)
> 	#define	PROC_PROFLOCK(p)	mtx_lock_spin(&(p)->p_profmtx)
>
> Let's find a bit of code that uses one, such as in kern_time.c:
>
> https://github.com/freebsd/freebsd/blob/master/sys/kern/kern_time.c#L338
>
> (kern_clock_gettime()).  This code reads:
>
> 	case CLOCK_PROF:
> 		PROC_LOCK(p);
> 		PROC_STATLOCK(p);
> 		calcru(p, &user, &sys);
> 		PROC_STATUNLOCK(p);
> 		PROC_UNLOCK(p);
> 		timevaladd(&user, &sys);
> 		TIMEVAL_TO_TIMESPEC(&user, ats);
> 		break;
>
> Note that the call to PROC_LOCK comes first, then the call to
> PROC_STATLOCK.  This is because PROC_LOCK
>
> https://github.com/freebsd/freebsd/blob/master/sys/sys/proc.h#L825
>
> is defined as:
>
> 	#define	PROC_LOCK(p)	mtx_lock(&(p)->p_mtx)
>
> If you obtain the locks in the other order -- i.e., if you grab
> the PROC_STATLOCK first, then try to lock PROC_LOCK -- you are
> trying to take a spin-type mutex while holding a default mutex,

Is this a typo? I guess you mean something like "you are trying
to take a blocking mutex while holding spin-type mutex".

> and this is not allowed (can cause deadlock).  But taking the
> PROC_LOCK first (which may block), then taking the PROC_STATLOCK
> (a spin lock) "inside" the outer PROC_LOCK default mutex, is OK.

I think I get your point: if you take a spin-type mutex, you
already disable interrupt, which in effect means that no other
code can preempt you. Under this circumstance, if you continue to
take a blocking mutex, you may get blocked. Since you already
disable interrupt and nobody can interrupt/preempt you, you are blocked
on that CPU, not being able to do anything, which is pretty much a
"deadlock" (actually this is not a deadlock, but, it is similar)

Regards,
Yubin Ruan

> (This is one of my mild objections to macros like PROC_LOCK and
> PROC_STATLOCK: they hide whether the mutex in question is a spin
> lock.)
>
> Incidentally, any time you take *any* lock while holding any
> other lock (e.g., lock A, then lock B while holding A), you have
> created a "lock order" in which A predeces B.  If some other
> code path locks B first, then while holding B, attempts to lock
> A, you get a deadlock if both code paths are running at the same
> time.  The WITNESS code dynamically discovers these various orders
> and warns you at run time if you have a "lock order reversal"
> (a case where one code path does A-then-B while another does
> B-then-A).
>
> (This is, in a sense, the same problem as discovering whether
> there is a loop in a directed graph, or whether this directed
> graph is acyclic.  If you can force the graph to take the shape of
> a tree, rather than the more general graph, there will never be
> any loops in it, and you will never have lock order reversals.
> And of course if you have only *one* lock for some data, there is
> nothing to be reversed.  Not all lock order reversals are
> guaranteed to lead to deadlock, but sorting out which ones are
> really OK, and which are not, is ... challenging.)
>
> Chris
>




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?aa5f22f4-0bec-f2a4-554b-f0055398eb7d>