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. >> >> For non-WITNESS< assertion support for SA_UNLOCKED was added in r125421 and >> made to panic in r126316. >> >> MFC after: 1 week >> >> Modified: >> head/share/man/man9/sx.9 >> >> Modified: head/share/man/man9/sx.9 >> ============================================================================== >> --- 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 correct > either. sx (and rw) locks do not track shared owners, so in non-witness > case only exclusive ownership by curthread triggers panic for SA_UNLOCKED > case. Shared ownership is silently ignored by the assert. >
Yes you're right. The original change for this in r125421 was checking
shared count:
> Index: head/sys/kern/kern_sx.c
> ===================================================================
> --- 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 != 0 && sx->sx_xholder == curthread)
> + 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, file,
> line);
But it was later removed in r126003:
> Index: head/sys/kern/kern_sx.c
> ===================================================================
> --- 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 != 0 && sx->sx_xholder == curthread)
> + if (sx->sx_xholder == 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.
--
Regards,
Bryan Drewery
signature.asc
Description: OpenPGP digital signature
