Messages in this thread | | | Date | Thu, 12 Oct 2017 16:54:35 +0900 | From | Byungchul Park <> | Subject | Re: [RESEND PATCH v2 2/2] lockdep: Remove unnecessary acquisitions wrt workqueue flush |
| |
On Tue, Oct 10, 2017 at 03:51:37PM +0900, Byungchul Park wrote: > The workqueue added manual acquisitions to catch deadlock cases. > Now crossrelease was introduced, some of those are redundant, since > wait_for_completion() already includes the acquisition for itself. > Removed it.
I think manual annotations for wait_for_completion() should be removed in this way, since it's already embedded in wait_for_completion(), now. Don't you think so?
> Signed-off-by: Byungchul Park <byungchul.park@lge.com> > --- > include/linux/workqueue.h | 4 ++-- > kernel/workqueue.c | 20 ++++---------------- > 2 files changed, 6 insertions(+), 18 deletions(-) > > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h > index db6dc9d..1bef13e 100644 > --- a/include/linux/workqueue.h > +++ b/include/linux/workqueue.h > @@ -218,7 +218,7 @@ static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { } > \ > __init_work((_work), _onstack); \ > (_work)->data = (atomic_long_t) WORK_DATA_INIT(); \ > - lockdep_init_map(&(_work)->lockdep_map, #_work, &__key, 0); \ > + lockdep_init_map(&(_work)->lockdep_map, "(complete)"#_work, &__key, 0); \ > INIT_LIST_HEAD(&(_work)->entry); \ > (_work)->func = (_func); \ > } while (0) > @@ -398,7 +398,7 @@ enum { > static struct lock_class_key __key; \ > const char *__lock_name; \ > \ > - __lock_name = #fmt#args; \ > + __lock_name = "(complete)"#fmt#args; \ > \ > __alloc_workqueue_key((fmt), (flags), (max_active), \ > &__key, __lock_name, ##args); \ > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index ab3c0dc..72f68b1 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -2497,15 +2497,8 @@ static void insert_wq_barrier(struct pool_workqueue *pwq, > INIT_WORK_ONSTACK(&barr->work, wq_barrier_func); > __set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&barr->work)); > > - /* > - * Explicitly init the crosslock for wq_barrier::done, make its lock > - * key a subkey of the corresponding work. As a result we won't > - * build a dependency between wq_barrier::done and unrelated work. > - */ > - lockdep_init_map_crosslock((struct lockdep_map *)&barr->done.map, > - "(complete)wq_barr::done", > - target->lockdep_map.key, 1); > - __init_completion(&barr->done); > + init_completion_with_map(&barr->done, &target->lockdep_map); > + > barr->task = current; > > /* > @@ -2611,16 +2604,14 @@ void flush_workqueue(struct workqueue_struct *wq) > struct wq_flusher this_flusher = { > .list = LIST_HEAD_INIT(this_flusher.list), > .flush_color = -1, > - .done = COMPLETION_INITIALIZER_ONSTACK(this_flusher.done), > }; > int next_color; > > + init_completion_with_map(&this_flusher.done, &wq->lockdep_map); > + > if (WARN_ON(!wq_online)) > return; > > - lock_map_acquire(&wq->lockdep_map); > - lock_map_release(&wq->lockdep_map); > - > mutex_lock(&wq->mutex); > > /* > @@ -2883,9 +2874,6 @@ bool flush_work(struct work_struct *work) > if (WARN_ON(!wq_online)) > return false; > > - lock_map_acquire(&work->lockdep_map); > - lock_map_release(&work->lockdep_map); > - > if (start_flush_work(work, &barr)) { > wait_for_completion(&barr.done); > destroy_work_on_stack(&barr.work); > -- > 1.9.1
| |