aboutsummaryrefslogtreecommitdiff
path: root/sys/sys
diff options
context:
space:
mode:
authorAlex Richardson <arichardson@FreeBSD.org>2021-02-18 10:25:10 +0000
committerAlex Richardson <arichardson@FreeBSD.org>2021-02-18 14:02:48 +0000
commitfa2528ac643519072c498b483d0dcc1fa5d99bc1 (patch)
tree255d2d54811e94f63b72fe40c4a88b4f11978e9f /sys/sys
parentdf093aa9463b2121d8307fb91c4ba7cf17f4ea64 (diff)
downloadsrc-fa2528ac643519072c498b483d0dcc1fa5d99bc1.tar.gz
src-fa2528ac643519072c498b483d0dcc1fa5d99bc1.zip
Use atomic loads/stores when updating td->td_state
KCSAN complains about racy accesses in the locking code. Those races are fine since they are inside a TD_SET_RUNNING() loop that expects the value to be changed by another CPU. Use relaxed atomic stores/loads to indicate that this variable can be written/read by multiple CPUs at the same time. This will also prevent the compiler from doing unexpected re-ordering. Reported by: GENERIC-KCSAN Test Plan: KCSAN no longer complains, kernel still runs fine. Reviewed By: markj, mjg (earlier version) Differential Revision: https://reviews.freebsd.org/D28569
Diffstat (limited to 'sys/sys')
-rw-r--r--sys/sys/proc.h34
1 files changed, 23 insertions, 11 deletions
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 99257878c2e0..0073fee2a42e 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -347,6 +347,7 @@ struct thread {
TDS_RUNQ,
TDS_RUNNING
} td_state; /* (t) thread state */
+ /* Note: td_state must be accessed using TD_{GET,SET}_STATE(). */
union {
register_t tdu_retval[2];
off_t tdu_off;
@@ -540,10 +541,15 @@ do { \
#define TD_IS_SWAPPED(td) ((td)->td_inhibitors & TDI_SWAPPED)
#define TD_ON_LOCK(td) ((td)->td_inhibitors & TDI_LOCK)
#define TD_AWAITING_INTR(td) ((td)->td_inhibitors & TDI_IWAIT)
-#define TD_IS_RUNNING(td) ((td)->td_state == TDS_RUNNING)
-#define TD_ON_RUNQ(td) ((td)->td_state == TDS_RUNQ)
-#define TD_CAN_RUN(td) ((td)->td_state == TDS_CAN_RUN)
-#define TD_IS_INHIBITED(td) ((td)->td_state == TDS_INHIBITED)
+#ifdef _KERNEL
+#define TD_GET_STATE(td) atomic_load_int(&(td)->td_state)
+#else
+#define TD_GET_STATE(td) ((td)->td_state)
+#endif
+#define TD_IS_RUNNING(td) (TD_GET_STATE(td) == TDS_RUNNING)
+#define TD_ON_RUNQ(td) (TD_GET_STATE(td) == TDS_RUNQ)
+#define TD_CAN_RUN(td) (TD_GET_STATE(td) == TDS_CAN_RUN)
+#define TD_IS_INHIBITED(td) (TD_GET_STATE(td) == TDS_INHIBITED)
#define TD_ON_UPILOCK(td) ((td)->td_flags & TDF_UPIBLOCKED)
#define TD_IS_IDLETHREAD(td) ((td)->td_flags & TDF_IDLETD)
@@ -557,15 +563,15 @@ do { \
((td)->td_inhibitors & TDI_LOCK) != 0 ? "blocked" : \
((td)->td_inhibitors & TDI_IWAIT) != 0 ? "iwait" : "yielding")
-#define TD_SET_INHIB(td, inhib) do { \
- (td)->td_state = TDS_INHIBITED; \
- (td)->td_inhibitors |= (inhib); \
+#define TD_SET_INHIB(td, inhib) do { \
+ TD_SET_STATE(td, TDS_INHIBITED); \
+ (td)->td_inhibitors |= (inhib); \
} while (0)
#define TD_CLR_INHIB(td, inhib) do { \
if (((td)->td_inhibitors & (inhib)) && \
(((td)->td_inhibitors &= ~(inhib)) == 0)) \
- (td)->td_state = TDS_CAN_RUN; \
+ TD_SET_STATE(td, TDS_CAN_RUN); \
} while (0)
#define TD_SET_SLEEPING(td) TD_SET_INHIB((td), TDI_SLEEPING)
@@ -581,9 +587,15 @@ do { \
#define TD_CLR_SUSPENDED(td) TD_CLR_INHIB((td), TDI_SUSPENDED)
#define TD_CLR_IWAIT(td) TD_CLR_INHIB((td), TDI_IWAIT)
-#define TD_SET_RUNNING(td) (td)->td_state = TDS_RUNNING
-#define TD_SET_RUNQ(td) (td)->td_state = TDS_RUNQ
-#define TD_SET_CAN_RUN(td) (td)->td_state = TDS_CAN_RUN
+#ifdef _KERNEL
+#define TD_SET_STATE(td, state) atomic_store_int(&(td)->td_state, state)
+#else
+#define TD_SET_STATE(td, state) (td)->td_state = state
+#endif
+#define TD_SET_RUNNING(td) TD_SET_STATE(td, TDS_RUNNING)
+#define TD_SET_RUNQ(td) TD_SET_STATE(td, TDS_RUNQ)
+#define TD_SET_CAN_RUN(td) TD_SET_STATE(td, TDS_CAN_RUN)
+
#define TD_SBDRY_INTR(td) \
(((td)->td_flags & (TDF_SEINTR | TDF_SERESTART)) != 0)