Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Aug 2011 08:00:07 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Andriy Gapon <avg@freebsd.org>
Cc:        freebsd-arch@freebsd.org, Hans Petter Selasky <hselasky@c2i.net>
Subject:   Re: skipping locks, mutex_owned, usb
Message-ID:  <201108250800.07467.jhb@freebsd.org>
In-Reply-To: <4E563354.5020106@FreeBSD.org>
References:  <4E53986B.5000804@FreeBSD.org> <201108251235.15853.hselasky@c2i.net> <4E563354.5020106@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, August 25, 2011 7:34:44 am Andriy Gapon wrote:
> on 25/08/2011 13:35 Hans Petter Selasky said the following:
> > On Tuesday 23 August 2011 15:11:08 John Baldwin wrote:
> >>> I.  [Why] do we need this pattern?
> >>> Can the code be re-written in a smarter (if not to say proper) way?
> >>
> > 
> > Hi,
> > 
> >> Giant is recursive, it should just be always acquired.  Also, this
> >> recursive call pattern is very non-obvious.  A far more straightforward
> >> approach would be to just have:
> > 
> > Unless Witness is changed, that won't work. It will lead to LOR warnings I 
> > think.
> > 
> > Imagine that the root function locks Giant, then another lock is locked and 
> > then ukbd_poll() is called. Won't the second lock of Giant cause a LOR 
> > warning?
> 
> I think no.
> At least that's how I interpret the following snippet in witness_checkorder():
> /*
>  * Check to see if we are recursing on a lock we already own.  If
>  * so, make sure that we don't mismatch exclusive and shared lock
>  * acquires.
>  */
> lock1 = find_instance(lock_list, lock);
> if (lock1 != NULL) {
>         if ((lock1->li_flags & LI_EXCLUSIVE) != 0 &&
>             (flags & LOP_EXCLUSIVE) == 0) {
>                 printf("shared lock of (%s) %s @ %s:%d\n",
>                     class->lc_name, lock->lo_name, file, line);
>                 printf("while exclusively locked from %s:%d\n",
>                     lock1->li_file, lock1->li_line);
>                 panic("share->excl");
>         }
>         if ((lock1->li_flags & LI_EXCLUSIVE) == 0 &&
>             (flags & LOP_EXCLUSIVE) != 0) {
>                 printf("exclusive lock of (%s) %s @ %s:%d\n",
>                     class->lc_name, lock->lo_name, file, line);
>                 printf("while share locked from %s:%d\n",
>                     lock1->li_file, lock1->li_line);
>                 panic("excl->share");
>         }
>         return;
> }
> 
> Because of the return statement we do not seem to be doing any additional order
> checking in the case of recursion.

Correct.  WITNESS has never done LOR checking on trylocks or recursive
acquires since those will never block.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108250800.07467.jhb>