Skip to content

Commit bc8b50c

Browse files
committed
workqueue: Drop the special locking rule for worker->flags and worker_pool->flags
worker->flags used to be accessed from scheduler hooks without grabbing pool->lock for concurrency management. This is no longer true since 6d25be5 ("sched/core, workqueues: Distangle worker accounting from rq lock"). Also, it's unclear why worker_pool->flags was using the "X" rule. All relevant users are accessing it under the pool lock. Let's drop the special "X" rule and use the "L" rule for these flag fields instead. While at it, replace the CONTEXT comment with lockdep_assert_held(). This allows worker_set/clr_flags() to be used from context which isn't the worker itself. This will be used later to implement assinging work items to workers before waking them up so that workqueue can have better control over which worker executes which work item on which CPU. The only actual changes are sanity checks. There shouldn't be any visible behavior changes. Signed-off-by: Tejun Heo <[email protected]>
1 parent 8743765 commit bc8b50c

File tree

2 files changed

+4
-15
lines changed

2 files changed

+4
-15
lines changed

kernel/workqueue.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,6 @@ enum {
122122
*
123123
* L: pool->lock protected. Access with pool->lock held.
124124
*
125-
* X: During normal operation, modification requires pool->lock and should
126-
* be done only from local cpu. Either disabling preemption on local
127-
* cpu or grabbing pool->lock is enough for read access. If
128-
* POOL_DISASSOCIATED is set, it's identical to L.
129-
*
130125
* K: Only modified by worker while holding pool->lock. Can be safely read by
131126
* self, while holding pool->lock or from IRQ context if %current is the
132127
* kworker.
@@ -160,7 +155,7 @@ struct worker_pool {
160155
int cpu; /* I: the associated cpu */
161156
int node; /* I: the associated node ID */
162157
int id; /* I: pool ID */
163-
unsigned int flags; /* X: flags */
158+
unsigned int flags; /* L: flags */
164159

165160
unsigned long watchdog_ts; /* L: watchdog timestamp */
166161
bool cpu_stall; /* WD: stalled cpu bound pool */
@@ -910,15 +905,12 @@ static void wake_up_worker(struct worker_pool *pool)
910905
* @flags: flags to set
911906
*
912907
* Set @flags in @worker->flags and adjust nr_running accordingly.
913-
*
914-
* CONTEXT:
915-
* raw_spin_lock_irq(pool->lock)
916908
*/
917909
static inline void worker_set_flags(struct worker *worker, unsigned int flags)
918910
{
919911
struct worker_pool *pool = worker->pool;
920912

921-
WARN_ON_ONCE(worker->task != current);
913+
lockdep_assert_held(&pool->lock);
922914

923915
/* If transitioning into NOT_RUNNING, adjust nr_running. */
924916
if ((flags & WORKER_NOT_RUNNING) &&
@@ -935,16 +927,13 @@ static inline void worker_set_flags(struct worker *worker, unsigned int flags)
935927
* @flags: flags to clear
936928
*
937929
* Clear @flags in @worker->flags and adjust nr_running accordingly.
938-
*
939-
* CONTEXT:
940-
* raw_spin_lock_irq(pool->lock)
941930
*/
942931
static inline void worker_clr_flags(struct worker *worker, unsigned int flags)
943932
{
944933
struct worker_pool *pool = worker->pool;
945934
unsigned int oflags = worker->flags;
946935

947-
WARN_ON_ONCE(worker->task != current);
936+
lockdep_assert_held(&pool->lock);
948937

949938
worker->flags &= ~flags;
950939

kernel/workqueue_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ struct worker {
4848
/* A: runs through worker->node */
4949

5050
unsigned long last_active; /* K: last active timestamp */
51-
unsigned int flags; /* X: flags */
51+
unsigned int flags; /* L: flags */
5252
int id; /* I: worker id */
5353

5454
/*

0 commit comments

Comments
 (0)