On Mon, May 29, 2017 at 3:33 AM, Johannes Berg <johan...@sipsolutions.net> wrote: > Hi Tejun, > > I suspect this is a long-standing bug introduced by all the pool rework > you did at some point, but I don't really know nor can I figure out how > to fix it right now. I guess it could possibly also be a lockdep issue, > or an issue in how it's used, but I definitely know that this used to > work (i.e. warn) back when I introduced the lockdep checking to the WQ > code. I was actually bitten by a bug like this, and erroneously > dismissed it as not being the case because lockdep hadn't warned (and > the actual deadlock debug output is basically not existent). > > In any case, the following code really should result in a warning from > lockdep, but doesn't. If you comment in the #define DEADLOCK, it will > actually cause a deadlock :) > > > #include <linux/kernel.h> > #include <linux/mutex.h> > #include <linux/workqueue.h> > #include <linux/module.h> > #include <linux/delay.h> > > DEFINE_MUTEX(mtx); > static struct workqueue_struct *wq; > static struct work_struct w1, w2; > > static void w1_wk(struct work_struct *w) > { > mutex_lock(&mtx); > msleep(100); > mutex_unlock(&mtx); > } > > static void w2_wk(struct work_struct *w) > { > } > > /* > * if not defined, then lockdep should warn only,
I guess when DEADLOCK not defined, there is no work is queued nor executed, therefore, no lock dependence is recorded, and there is no warn either. > * if defined, the system will really deadlock. > */ > > //#define DEADLOCK > > static int init(void) > { > wq = create_singlethread_workqueue("test"); > if (!wq) > return -ENOMEM; > INIT_WORK(&w1, w1_wk); > INIT_WORK(&w2, w2_wk); > /* add lock dependence, the lockdep should warn */ queue_work(wq, &w1); queue_work(wq, &w2); flush_work(&w1); > #ifdef DEADLOCK > queue_work(wq, &w1); > queue_work(wq, &w2); > #endif > mutex_lock(&mtx); > flush_work(&w2); > mutex_unlock(&mtx); > > #ifndef DEADLOCK > queue_work(wq, &w1); > queue_work(wq, &w2); > #endif > > return 0; > } > module_init(init); > > > (to test, just copy it to some C file and add "obj-y += myfile.o" to > the Makefile in that directory, then boot the kernel - perhaps in a VM) > > johannes