Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1655943

Re: single-threaded wq lockdep is broken

From Lai Jiangshan <jiangshanlai@gmail.com>
Newsgroups linux.kernel
Subject Re: single-threaded wq lockdep is broken
Date 2017-06-02 09:10 +0200
Message-ID <tNOSD-698-35@gated-at.bofh.it> (permalink)
References <tMccF-6e4-1@gated-at.bofh.it> <tN7kD-2mN-33@gated-at.bofh.it> <tN7kD-2mN-35@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

single-threaded wq lockdep is broken Johannes Berg <johannes@sipsolutions.net> - 2017-05-28 21:40 +0200
  Re: single-threaded wq lockdep is broken Lai Jiangshan <jiangshanlai@gmail.com> - 2017-05-31 10:40 +0200
    Re: single-threaded wq lockdep is broken Johannes Berg <johannes@sipsolutions.net> - 2017-05-31 10:40 +0200
      Re: single-threaded wq lockdep is broken Johannes Berg <johannes@sipsolutions.net> - 2017-05-31 10:50 +0200
      Re: single-threaded wq lockdep is broken Lai Jiangshan <jiangshanlai@gmail.com> - 2017-06-02 09:10 +0200
        Re: single-threaded wq lockdep is broken Johannes Berg <johannes@sipsolutions.net> - 2017-06-02 11:00 +0200
  Re: single-threaded wq lockdep is broken Tejun Heo <tj@kernel.org> - 2017-05-31 20:30 +0200
    Re: single-threaded wq lockdep is broken Johannes Berg <johannes@sipsolutions.net> - 2017-05-31 21:20 +0200

csiph-web