NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/57751: cv_signal and cv_broadcast without mutex



>Number:         57751
>Category:       kern
>Synopsis:       cv_signal and cv_broadcast without mutex
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Dec 04 17:45:00 +0000 2023
>Originator:     Ruslan Nikolaev
>Release:        10.0
>Organization:
Penn State University
>Environment:
>Description:
It seems that recently the semantics of cv_signal and cv_broadcast was changed: https://www.mail-archive.com/source-changes-d%netbsd.org@localhost/msg59675.html

 The mutex passed to the wait function
 .Po Fa mtx Pc
-must also be held when calling
-.Fn cv_signal .
+should be held or have been released immediately before
+.Fn cv_signal
+is called.

Is there any good reason for this change and allowing to NOT hold the mutex? In my opinion, requiring the mutex to be always locked is a very good idea since it typically leads to better and more predictable performance (e.g., the condition is less likely to change by the time cv_wait wakes up, i.e., less likely to have spurious wake ups if someone is able to grab the mutex before cv_signal is sent and changes the condition). In addition, POSIX recommends holding the mutex for more predictable scheduling: https://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_cond_signal.html

Also, it forces the programmer to think carefully what mutexes are being used in cv_wait, etc.

I actually prefer *requiring* rather than just recommending to always hold the mutex because it can enable additional optimizations. For example, in rumprun-smp, we were able to piggyback on the (implicit) mutex (assuming that it is still locked when cv_signal is called) to manipulate a blocking queue without having another lock:
https://github.com/ssrg-vt/rumprun-smp/blob/master/lib/libbmk_rumpuser/rumpuser_synch.c
>How-To-Repeat:

>Fix:



Home | Main Index | Thread Index | Old Index