Re: single-threaded wq lockdep is broken

Lai Jiangshan
2017-06-02T07:04:01Z

On Wed, May 31, 2017 at 4:36 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> Hi,
>
>> > #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
>
> This was "ifndef", so it does in fact run here, just like you
> suggested. It doesn't warn though.
>
> I don't think the order of queue/flush would matter, in fact, if you
> insert it like you did, with the flush outside the mutex, no issue
> exists (until the later flush)
>

the @w2 is not queued before flush_work(&w2), it is expected
that @w2 is not associated with @wq, and the dependence
mtx -> wq will not be recorded. And it is expected no warning.

> Also, even if DEADLOCK *is* defined, lockdep doesn't report anything.

Uhhh..... I have no idea about it yet.