Skip to content

Commit

Permalink
workqueue: skip lockdep wq dependency in cancel_work_sync()
Browse files Browse the repository at this point in the history
In cancel_work_sync(), we can only have one of two cases, even
with an ordered workqueue:
 * the work isn't running, just cancelled before it started
 * the work is running, but then nothing else can be on the
   workqueue before it

Thus, we need to skip the lockdep workqueue dependency handling,
otherwise we get false positive reports from lockdep saying that
we have a potential deadlock when the workqueue also has other
work items with locking, e.g.

  work1_function() { mutex_lock(&mutex); ... }
  work2_function() { /* nothing */ }

  other_function() {
    queue_work(ordered_wq, &work1);
    queue_work(ordered_wq, &work2);
    mutex_lock(&mutex);
    cancel_work_sync(&work2);
  }

As described above, this isn't a problem, but lockdep will
currently flag it as if cancel_work_sync() was flush_work(),
which *is* a problem.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
  • Loading branch information
Johannes Berg authored and Tejun Heo committed Aug 22, 2018
1 parent 66448bc commit d6e8978
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions kernel/workqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,8 @@ void drain_workqueue(struct workqueue_struct *wq)
}
EXPORT_SYMBOL_GPL(drain_workqueue);

static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr)
static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
bool from_cancel)
{
struct worker *worker = NULL;
struct worker_pool *pool;
Expand Down Expand Up @@ -2885,7 +2886,8 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr)
* workqueues the deadlock happens when the rescuer stalls, blocking
* forward progress.
*/
if (pwq->wq->saved_max_active == 1 || pwq->wq->rescuer) {
if (!from_cancel &&
(pwq->wq->saved_max_active == 1 || pwq->wq->rescuer)) {
lock_map_acquire(&pwq->wq->lockdep_map);
lock_map_release(&pwq->wq->lockdep_map);
}
Expand All @@ -2896,6 +2898,22 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr)
return false;
}

static bool __flush_work(struct work_struct *work, bool from_cancel)
{
struct wq_barrier barr;

if (WARN_ON(!wq_online))
return false;

if (start_flush_work(work, &barr, from_cancel)) {
wait_for_completion(&barr.done);
destroy_work_on_stack(&barr.work);
return true;
} else {
return false;
}
}

/**
* flush_work - wait for a work to finish executing the last queueing instance
* @work: the work to flush
Expand All @@ -2909,18 +2927,7 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr)
*/
bool flush_work(struct work_struct *work)
{
struct wq_barrier barr;

if (WARN_ON(!wq_online))
return false;

if (start_flush_work(work, &barr)) {
wait_for_completion(&barr.done);
destroy_work_on_stack(&barr.work);
return true;
} else {
return false;
}
return __flush_work(work, false);
}
EXPORT_SYMBOL_GPL(flush_work);

Expand Down Expand Up @@ -2986,7 +2993,7 @@ static bool __cancel_work_timer(struct work_struct *work, bool is_dwork)
* isn't executing.
*/
if (wq_online)
flush_work(work);
__flush_work(work, true);

clear_work_data(work);

Expand Down

0 comments on commit d6e8978

Please sign in to comment.