diff mbox series

[2/2] workqueue: create lockdep dependency in flush_work()

Message ID 20180821120317.4115-3-johannes@sipsolutions.net (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show
Series workqueue lockdep limitations/bugs | expand

Commit Message

Johannes Berg Aug. 21, 2018, 12:03 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

In flush_work(), we need to create a lockdep dependency so that
the following scenario is appropriately tagged as a problem:

  work_function()
  {
    mutex_lock(&mutex);
    ...
  }

  other_function()
  {
    mutex_lock(&mutex);
    flush_work(&work); // or cancel_work_sync(&work);
  }

This is a problem since the work might be running and be blocked
on trying to acquire the mutex.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 kernel/workqueue.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Tejun Heo Aug. 21, 2018, 4:09 p.m. UTC | #1 
On Tue, Aug 21, 2018 at 02:03:17PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> In flush_work(), we need to create a lockdep dependency so that
> the following scenario is appropriately tagged as a problem:
> 
>   work_function()
>   {
>     mutex_lock(&mutex);
>     ...
>   }
> 
>   other_function()
>   {
>     mutex_lock(&mutex);
>     flush_work(&work); // or cancel_work_sync(&work);
>   }
> 
> This is a problem since the work might be running and be blocked
> on trying to acquire the mutex.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

This makes sense to me.  Did you notice any extra lockdep warnings
with this?

Thanks.
Johannes Berg Aug. 21, 2018, 5:19 p.m. UTC | #2 
On Tue, 2018-08-21 at 09:09 -0700, Tejun Heo wrote:
> 
> This makes sense to me.  Did you notice any extra lockdep warnings
> with this?

I haven't used it beyond testing the trivial example I created on the
spot with a small mac80211 change, so can't really say either way.

johannes
diff mbox series

Patch

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index a6c2b823f348..29d4d84ba512 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2852,6 +2852,11 @@  static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
 
 	might_sleep();
 
+	if (!from_cancel) {
+		lock_map_acquire(&work->lockdep_map);
+		lock_map_release(&work->lockdep_map);
+	}
+
 	local_irq_disable();
 	pool = get_work_pool(work);
 	if (!pool) {