Skip to content

Commit

Permalink
workqueue: Drop the special locking rule for worker->flags and worker…
Browse files Browse the repository at this point in the history
…_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]>
  • Loading branch information
htejun committed Aug 8, 2023
1 parent 8743765 commit bc8b50c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 15 deletions.
17 changes: 3 additions & 14 deletions kernel/workqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,6 @@ enum {
*
* L: pool->lock protected. Access with pool->lock held.
*
* X: During normal operation, modification requires pool->lock and should
* be done only from local cpu. Either disabling preemption on local
* cpu or grabbing pool->lock is enough for read access. If
* POOL_DISASSOCIATED is set, it's identical to L.
*
* K: Only modified by worker while holding pool->lock. Can be safely read by
* self, while holding pool->lock or from IRQ context if %current is the
* kworker.
Expand Down Expand Up @@ -160,7 +155,7 @@ struct worker_pool {
int cpu; /* I: the associated cpu */
int node; /* I: the associated node ID */
int id; /* I: pool ID */
unsigned int flags; /* X: flags */
unsigned int flags; /* L: flags */

unsigned long watchdog_ts; /* L: watchdog timestamp */
bool cpu_stall; /* WD: stalled cpu bound pool */
Expand Down Expand Up @@ -910,15 +905,12 @@ static void wake_up_worker(struct worker_pool *pool)
* @flags: flags to set
*
* Set @flags in @worker->flags and adjust nr_running accordingly.
*
* CONTEXT:
* raw_spin_lock_irq(pool->lock)
*/
static inline void worker_set_flags(struct worker *worker, unsigned int flags)
{
struct worker_pool *pool = worker->pool;

WARN_ON_ONCE(worker->task != current);
lockdep_assert_held(&pool->lock);

/* If transitioning into NOT_RUNNING, adjust nr_running. */
if ((flags & WORKER_NOT_RUNNING) &&
Expand All @@ -935,16 +927,13 @@ static inline void worker_set_flags(struct worker *worker, unsigned int flags)
* @flags: flags to clear
*
* Clear @flags in @worker->flags and adjust nr_running accordingly.
*
* CONTEXT:
* raw_spin_lock_irq(pool->lock)
*/
static inline void worker_clr_flags(struct worker *worker, unsigned int flags)
{
struct worker_pool *pool = worker->pool;
unsigned int oflags = worker->flags;

WARN_ON_ONCE(worker->task != current);
lockdep_assert_held(&pool->lock);

worker->flags &= ~flags;

Expand Down
2 changes: 1 addition & 1 deletion kernel/workqueue_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct worker {
/* A: runs through worker->node */

unsigned long last_active; /* K: last active timestamp */
unsigned int flags; /* X: flags */
unsigned int flags; /* L: flags */
int id; /* I: worker id */

/*
Expand Down

0 comments on commit bc8b50c

Please sign in to comment.