Re: svn commit: r296947 - head/share/man/man9
On 3/16/2016 11:45 AM, Konstantin Belousov wrote:
> On Wed, Mar 16, 2016 at 06:39:48PM +, 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_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_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
svn commit: r296947 - head/share/man/man9
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.9Wed Mar 16 17:35:55 2016(r296946) +++ head/share/man/man9/sx.9Wed 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 ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r296947 - head/share/man/man9
On 3/16/2016 3:12 PM, John Baldwin wrote: > On Wednesday, March 16, 2016 06:39:48 PM 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 > Eh, how can this possibly work? Kib and I discussed it on here already. I have a fix in https://reviews.freebsd.org/D5659 -- Regards, Bryan Drewery signature.asc Description: OpenPGP digital signature
Re: svn commit: r296947 - head/share/man/man9
On Wednesday, March 16, 2016 06:39:48 PM 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
Eh, how can this possibly work?
That is, suppose I have this code:
sx_slock();
sx_assert(, SA_UNLOCKED);
How does that safely work without WITNESS? It needs to not panic in the case
that some other thread does sx_slock(). In fact, the comment (modulo the
spelling nit) says this explicitly:
case SA_UNLOCKED:
#ifdef WITNESS
witness_assert(>lock_object, what, file, line);
#else
/*
* If we hold an exclusve lock fail. We can't
* reliably check to see if we hold a shared lock or
* not.
*/
if (sx_xholder(sx) == curthread)
panic("Lock %s exclusively locked @ %s:%d\n",
sx->lock_object.lo_name, file, line);
#endif
break;
You could perhaps reword the bug to say that SA_UNLOCKED will panic if the lock
is exclusively locked in the non-WITNESS case, but will fail to panic if the
thread holds a shared lock.
The wording in rwlock(9) is better than sx(9) though it should mention that
RA_UNLOCKED can detect a write lock. That wording could then be used in sx(9).
--
John Baldwin
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r296947 - head/share/man/man9
On Wed, Mar 16, 2016 at 06:39:48PM +, 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. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
