lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 18 Sep 2017 17:46:22 +0900 From: Byungchul Park <byungchul.park@....com> To: tj@...nel.org, johannes.berg@...el.com, peterz@...radead.org, mingo@...nel.org Cc: tglx@...utronix.de, oleg@...hat.com, linux-kernel@...r.kernel.org, kernel-team@....com Subject: Re: [PATCH v2 2/2] lockdep: Remove unnecessary acquisitions wrt workqueue flush On Fri, Sep 08, 2017 at 05:46:25PM +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. Now, lock_map_acquire() is already embedded in wait_for_completion(), manual acquisitions for the same purpose should be removed, since they would be duplicated. Consider this, please. > Signed-off-by: Byungchul Park <byungchul.park@....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
Powered by blists - more mailing lists