Re: single-threaded wq lockdep is broken

[Lai Jiangshan]

On Wed, May 31, 2017 at 4:36 PM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> 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.