Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 16 Mar 2016 12:18:44 -0700
From:      Bryan Drewery <bdrewery@FreeBSD.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r296947 - head/share/man/man9
Message-ID:  <56E9B194.9070008@FreeBSD.org>
In-Reply-To: <20160316184545.GN1741@kib.kiev.ua>
References:  <201603161839.u2GIdm5C072960@repo.freebsd.org> <20160316184545.GN1741@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--o3LNveQ5VfxUmAWjgoSSPaRjwwWvb6juH
Content-Type: multipart/mixed; boundary="ToQuchvP8JMOeebp0q6g77at569GIK31N"
From: Bryan Drewery <bdrewery@FreeBSD.org>
To: Konstantin Belousov <kostikbel@gmail.com>
Cc: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Message-ID: <56E9B194.9070008@FreeBSD.org>
Subject: Re: svn commit: r296947 - head/share/man/man9
References: <201603161839.u2GIdm5C072960@repo.freebsd.org>
 <20160316184545.GN1741@kib.kiev.ua>
In-Reply-To: <20160316184545.GN1741@kib.kiev.ua>

--ToQuchvP8JMOeebp0q6g77at569GIK31N
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

On 3/16/2016 11:45 AM, Konstantin Belousov wrote:
> On Wed, Mar 16, 2016 at 06:39:48PM +0000, Bryan Drewery wrote:
>> Author: bdrewery
>> Date: Wed Mar 16 18:39:48 2016
>> New Revision: 296947
>> URL: https://svnweb.freebsd.org/changeset/base/296947
>>
>> Log:
>>   Remove incorrect BUGS entry about asserting lock not held.
>>  =20
>>   For non-WITNESS< assertion support for SA_UNLOCKED was added in r125=
421 and
>>   made to panic in r126316.
>>  =20
>>   MFC after:	1 week
>>
>> Modified:
>>   head/share/man/man9/sx.9
>>
>> Modified: head/share/man/man9/sx.9
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- head/share/man/man9/sx.9	Wed Mar 16 17:35:55 2016	(r296946)
>> +++ head/share/man/man9/sx.9	Wed Mar 16 18:39:48 2016	(r296947)
>> @@ -26,7 +26,7 @@
>>  .\"
>>  .\" $FreeBSD$
>>  .\"
>> -.Dd March 13, 2016
>> +.Dd March 16, 2016
>>  .Dt SX 9
>>  .Os
>>  .Sh NAME
>> @@ -320,11 +320,6 @@ end up sleeping while holding a mutex, w
>>  .Xr rwlock 9 ,
>>  .Xr sema 9
>>  .Sh BUGS
>> -Currently there is no way to assert that a lock is not held.
>> -This is not possible in the
>> -.No non- Ns Dv WITNESS
>> -case for asserting that this thread
>> -does not hold a shared lock.
>>  In the
>>  .No non- Ns Dv WITNESS
>>  case, the
> The removed text was not quite correct, but its removal is not quite co=
rrect
> either.  sx (and rw) locks do not track shared owners, so in non-witnes=
s
> case only exclusive ownership by curthread triggers panic for SA_UNLOCK=
ED
> case.  Shared ownership is silently ignored by the assert.
>=20

Yes you're right. The original change for this in r125421 was checking
shared count:

> Index: head/sys/kern/kern_sx.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- head/sys/kern/kern_sx.c     (revision 125420)
> +++ head/sys/kern/kern_sx.c     (revision 125421)
> @@ -344,6 +344,17 @@
>                             sx->sx_object.lo_name, file, line);
>                 mtx_unlock(sx->sx_lock);
>                 break;
> +       case SX_UNLOCKED:
> +#ifdef WITNESS
> +               witness_assert(&sx->sx_object, what, file, line);
> +#else
> +               mtx_lock(sx->sx_lock);
> +               if (sx->sx_cnt !=3D 0 && sx->sx_xholder =3D=3D curthrea=
d)
> +                       printf("Lock %s locked @ %s:%d\n",
> +                           sx->sx_object.lo_name, file, line);
> +               mtx_unlock(sx->sx_lock);
> +#endif
> +               break;
>         default:
>                 panic("Unknown sx lock assertion: %d @ %s:%d", what, fi=
le,
>                     line);

But it was later removed in r126003:

> Index: head/sys/kern/kern_sx.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- head/sys/kern/kern_sx.c     (revision 126002)
> +++ head/sys/kern/kern_sx.c     (revision 126003)
> @@ -348,8 +348,12 @@
>  #ifdef WITNESS
>                 witness_assert(&sx->sx_object, what, file, line);
>  #else
> +               /*
> +                * We are able to check only exclusive lock here,
> +                * we cannot assert that *this* thread owns slock.
> +                */
>                 mtx_lock(sx->sx_lock);
> -               if (sx->sx_cnt !=3D 0 && sx->sx_xholder =3D=3D curthrea=
d)
> +               if (sx->sx_xholder =3D=3D curthread)
>                         printf("Lock %s locked @ %s:%d\n",
>                             sx->sx_object.lo_name, file, line);
>                 mtx_unlock(sx->sx_lock);


I incorrectly was looking at only the original code and the panic fix,
rather than current code to ensure it still was checking shared lock
holders.

--=20
Regards,
Bryan Drewery


--ToQuchvP8JMOeebp0q6g77at569GIK31N--

--o3LNveQ5VfxUmAWjgoSSPaRjwwWvb6juH
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBAgAGBQJW6bGUAAoJEDXXcbtuRpfPDxUH/23sWb4+uLtL16rSS/HdehET
dmktp7mBOQ61Wvyj3MiU5fSLVtZHgHtsjMqRykNrfGkUT91d/Cki9w2X8OlitCgU
TMuCoT1OexBGSo6kFfqcH5zsqEFsSPvmVXB35x2jYQXEpihqUrlm3HjMDVDUpGlL
Ew0PRJv1riGc+jW+52IEHXquQ+luwRRN1KLTLYi+U+JcQbascRGRRi/R9ykx5tTd
mbBcDtX+ZCwJsewD70Yl7dbI5O4fJXi7fryiYOx6CXnLqK0YfDB3cO3zCoCvVPb6
brbyYUOrRTT0EU4yXWiuiqleQYsMCL5hgmAx30m6IYP0nEiD6q1MjuS09VyR1DY=
=zkR0
-----END PGP SIGNATURE-----

--o3LNveQ5VfxUmAWjgoSSPaRjwwWvb6juH--



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