svn commit: r273966 - in head: share/man/man9 sys/kern sys/sys

[Attilio Rao]

On Sun, Nov 2, 2014 at 5:59 PM, Konstantin Belousov <kostikbel at gmail.com> wrote:
> On Sun, Nov 02, 2014 at 05:42:55PM +0100, Attilio Rao wrote:
>> On Sun, Nov 2, 2014 at 5:37 PM, Konstantin Belousov <kostikbel at gmail.com> wrote:
>> > On Sun, Nov 02, 2014 at 04:54:46PM +0100, Attilio Rao wrote:
>> >> On Sun, Nov 2, 2014 at 2:10 PM, Konstantin Belousov <kib at freebsd.org> wrote:
>> >> > Author: kib
>> >> > Date: Sun Nov  2 13:10:31 2014
>> >> > New Revision: 273966
>> >> > URL: https://svnweb.freebsd.org/changeset/base/273966
>> >> >
>> >> > Log:
>> >> >   Fix two issues with lockmgr(9) LK_CAN_SHARE() test, which determines
>> >> >   whether the shared request for already shared-locked lock could be
>> >> >   granted.  Both problems result in the exclusive locker starvation.
>> >> >
>> >> >   The concurrent exclusive request is indicated by either
>> >> >   LK_EXCLUSIVE_WAITERS or LK_EXCLUSIVE_SPINNERS flags.  The reverse
>> >> >   condition, i.e. no exclusive waiters, must check that both flags are
>> >> >   cleared.
>> >> >
>> >> >   Add a flag LK_NODDLKTREAT for shared lock request to indicate that
>> >> >   current thread guarantees that it does not own the lock in shared
>> >> >   mode.  This turns back the exclusive lock starvation avoidance code;
>> >> >   see man page update for detailed description.
>> >> >
>> >> >   Use LK_NODDLKTREAT when doing lookup(9).
>> >>
>> >> The right way to implement this (selectively disabling writer
>> >> starvation avoidance) must be on a lock-basis.
>> >> So you need a new flag to pass to lockinit(). This is to support it "globaly".
>> > Any vnode (except some very special) does participate in lookup.
>> > So the proposed new flag has to be passed to every lockmgr init call.
>> > Disabling exclusive starvation support for all vnode lock calls is
>> > also wrong.
>> >
>> >> Then, you can pass it on a lockmgr() instance basis (like FreeBSD also
>> >> passes, for example, LK_NOWITNESS).
>> >> You can use it during lookup() calls. Maybe you will need to propagate
>> >> it via the vn_lock() interface.
>> > Well, if you indeed looked at the patch, you would note that this is
>> > exactly what is done.  The flag is passed only to vn_lock() calls
>> > which are coming from lookup (VOP_LOOKUP as well).
>> >
>> > The flag use is safe since deadlock prevention by td_lk_slocks only
>> > matters when the same lock was previously taken by the same thread
>> > which does recursive shared request [*].  td_lk_slocks is the poor and
>> > very indiscriminative way to express this, but might be unavoidable
>> > since shared owners are not recorded.
>>
>> If you have a better way to fix this into a "rich and discriminative"
>> way I'm all ears.
> It is easy and cheap to record the set of the owned lockmgr locks for
> current thread. I do not believe that we have a situation where more
> than 3 locks are held in one time. To give it some slack, we can record
> 8 locks shared-owned and do the check for recursion in LK_CAN_SHARE().
> If the thread-locks table overflows, we could either panic() or fall
> back to indiscriminative deadlock avoidance (i.e. relying on the sole
> td_lk_slocks value).

I don't think it is going to be cheap (and certainly it is not viable
for things like sx locks and rwlocks).
Checking for the owner chain anytime you acquire the lock is certainly
not I would call cheap.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein