From: Tejun Heo <tj@kernel.org>
To: Ingo Molnar <mingo@redhat.com>, "Rafael J. Wysocki" <rjw@sisk.pl>,
linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH] workqueue: relax lockdep annotation on flush_work()
Date: Wed, 29 Dec 2010 13:57:11 +0100 [thread overview]
Message-ID: <20101229125711.GL488@htj.dyndns.org> (raw)
Currently, the lockdep annotation in flush_work() requires exclusive
access on the workqueue the target work is queued on and triggers
warning if a work is trying to flush another work on the same
workqueue; however, this is no longer true as workqueues can now
execute multiple works concurrently.
This patch adds lock_map_acquire_read() and make process_one_work()
hold read access to the workqueue while executing a work and
start_flush_work() check for write access if concurrnecy level is one
and read access if higher.
This better represents what's going on and removes spurious lockdep
warnings which are triggered by fake dependency chain created through
flush_work().
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: "Rafael J. Wysocki" <rjw@sisk.pl>
---
How should this one be routed? The lockdep part can be split, merged
back into workqueue tree and so on but that seems a bit too much. If
it's okay, I'll route this through the workqueue tree. Going through
the lockdep tree is fine too.
Thanks.
include/linux/lockdep.h | 3 +++
kernel/workqueue.c | 8 ++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 71c09b2..9f19430 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -522,12 +522,15 @@ static inline void print_irqtrace_events(struct task_struct *curr)
#ifdef CONFIG_DEBUG_LOCK_ALLOC
# ifdef CONFIG_PROVE_LOCKING
# define lock_map_acquire(l) lock_acquire(l, 0, 0, 0, 2, NULL, _THIS_IP_)
+# define lock_map_acquire_read(l) lock_acquire(l, 0, 0, 2, 2, NULL, _THIS_IP_)
# else
# define lock_map_acquire(l) lock_acquire(l, 0, 0, 0, 1, NULL, _THIS_IP_)
+# define lock_map_acquire_read(l) lock_acquire(l, 0, 0, 2, 1, NULL, _THIS_IP_)
# endif
# define lock_map_release(l) lock_release(l, 1, _THIS_IP_)
#else
# define lock_map_acquire(l) do { } while (0)
+# define lock_map_acquire_read(l) do { } while (0)
# define lock_map_release(l) do { } while (0)
#endif
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 8ee6ec8..85f8f7b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1840,7 +1840,7 @@ __acquires(&gcwq->lock)
spin_unlock_irq(&gcwq->lock);
work_clear_pending(work);
- lock_map_acquire(&cwq->wq->lockdep_map);
+ lock_map_acquire_read(&cwq->wq->lockdep_map);
lock_map_acquire(&lockdep_map);
trace_workqueue_execute_start(work);
f(work);
@@ -2384,8 +2384,12 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
insert_wq_barrier(cwq, barr, work, worker);
spin_unlock_irq(&gcwq->lock);
- lock_map_acquire(&cwq->wq->lockdep_map);
+ if (cwq->wq->saved_max_active > 1)
+ lock_map_acquire_read(&cwq->wq->lockdep_map);
+ else
+ lock_map_acquire(&cwq->wq->lockdep_map);
lock_map_release(&cwq->wq->lockdep_map);
+
return true;
already_gone:
spin_unlock_irq(&gcwq->lock);
next reply other threads:[~2010-12-29 12:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-29 12:57 Tejun Heo [this message]
2010-12-29 14:20 ` [PATCH] workqueue: relax lockdep annotation on flush_work() Rafael J. Wysocki
2011-01-03 9:31 ` Peter Zijlstra
2011-01-03 14:17 ` [PATCH UPDATED] " Tejun Heo
2011-01-03 14:54 ` Peter Zijlstra
2011-01-03 15:00 ` Tejun Heo
2011-01-03 15:14 ` Peter Zijlstra
2011-01-03 15:20 ` Tejun Heo
2011-01-03 15:29 ` Peter Zijlstra
2011-01-03 21:25 ` Rafael J. Wysocki
2011-01-09 22:34 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101229125711.GL488@htj.dyndns.org \
--to=tj@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rjw@sisk.pl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox