Messages in this thread | | | Date | Mon, 6 Aug 2007 14:53:16 +0400 | From | Oleg Nesterov <> | Subject | Re: [NFS] 2.6.23-rc1-mm2 |
| |
On 08/06, Johannes Berg wrote: > > On Fri, 2007-08-03 at 21:21 +0400, Oleg Nesterov wrote: > > > To avoid a possible confusion: it is still OK if work->func() flushes > > its own workqueue, so strictly speaking this trace is false positive, > > but it would be very nice if we can get rid of this practice. > > However, the question whether we should allow flush_workqueue from > within a struct work is mainly an API policy issue; it doesn't hurt to > flush a workqueue from within a work,
I am not sure, but currently I hope we can forbid this eventually, so I personally think it is good that your patch complains.
> --- wireless-dev.orig/kernel/workqueue.c 2007-08-06 08:11:23.297846657 +0200 > +++ wireless-dev/kernel/workqueue.c 2007-08-06 08:19:54.727846657 +0200 > @@ -272,7 +272,7 @@ static void run_workqueue(struct cpu_wor > > BUG_ON(get_wq_data(work) != cwq); > work_clear_pending(work); > - lock_acquire(&cwq->wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&cwq->wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_acquire(&lockdep_map, 0, 0, 0, 2, _THIS_IP_); > f(work); > lock_release(&lockdep_map, 1, _THIS_IP_); > @@ -395,7 +395,7 @@ void fastcall flush_workqueue(struct wor > int cpu; > > might_sleep(); > - lock_acquire(&wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_release(&wq->lockdep_map, 1, _THIS_IP_); > for_each_cpu_mask(cpu, *cpu_map) > flush_cpu_workqueue(per_cpu_ptr(wq->cpu_wq, cpu)); > @@ -779,7 +779,7 @@ static void cleanup_workqueue_thread(str > if (cwq->thread == NULL) > return; > > - lock_acquire(&cwq->wq->lockdep_map, 0, 0, 0, 2, _THIS_IP_); > + lock_acquire(&cwq->wq->lockdep_map, 0, 0, 1, 2, _THIS_IP_); > lock_release(&cwq->wq->lockdep_map, 1, _THIS_IP_); > > flush_cpu_workqueue(cwq);
But this makes ->lockdep_map meaningless? We always take wq->lockdep_map for reading, now we can't detect deadlocks.
read_lock(A); lock(B);
vs lock(B); read_lock(A);
is valid, kernel/lockdep.c should not complain.
No?
Oleg.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |