All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Christian Lamparter <chunkeey@googlemail.com>,
	Andrey Konovalov <andreyknvl@google.com>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	linux-wireless@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Kostya Serebryany <kcc@google.com>,
	syzkaller <syzkaller@googlegroups.com>,
	Stephen Boyd <sboyd@codeaurora.org>, Tejun Heo <tj@kernel.org>,
	Yong Zhang <yong.zhang0@gmail.com>
Subject: Re: usb/net/p54: trying to register non-static key in p54_unregister_leds
Date: Wed, 20 Sep 2017 21:55:07 +0200	[thread overview]
Message-ID: <1505937307.3026.20.camel@sipsolutions.net> (raw)
In-Reply-To: <2277141.bYDD1vAb9W@debian64> (sfid-20170920_212816_631524_D372B13F)

On Wed, 2017-09-20 at 21:27 +0200, Christian Lamparter wrote:

> It seems this is caused as a result of:
>     -> lock_map_acquire(&work->lockdep_map);
> 	    lock_map_release(&work->lockdep_map);
> 
>     in flush_work() [0]

Agree.

> This was added by:
> 
> 	commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
> 	Author: Stephen Boyd <sboyd@codeaurora.org>
> 	Date:   Fri Apr 20 17:28:50 2012 -0700
> 
> 	workqueue: Catch more locking problems with flush_work()

Yes, but that doesn't matter.
    
> Looking at the Stephen's patch, it's clear that it was made
> with "static DECLARE_WORK(work, my_work)" in mind. However
> p54's led_work is "per-device", hence it is stored in the
> devices context p54_common, which is dynamically allocated.
> So, maybe revert Stephen's patch?

I disagree - as the lockdep warning says:

> > INFO: trying to register non-static key.
> > the code is fine but needs lockdep annotation.
> > turning off the locking correctness validator.

What it needs is to actually correctly go through initializing the work
at least once.

Without more information, I can't really say what's going on, but I
assume that something is failing and p54_unregister_leds() is getting
invoked without p54_init_leds() having been invoked, so essentially
it's trying to flush a work that was never initialized?

INIT_DELAYED_WORK() does, after all, initialize the lockdep map
properly via __INIT_WORK().

johannes

  reply	other threads:[~2017-09-20 19:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-20 18:37 usb/net/p54: trying to register non-static key in p54_unregister_leds Andrey Konovalov
2017-09-20 19:27 ` Christian Lamparter
2017-09-20 19:55   ` Johannes Berg [this message]
2017-09-21 18:22     ` Andrey Konovalov
2017-09-23 19:37       ` [RESEND] " Christian Lamparter
2017-09-24 14:13         ` Johannes Berg
2017-09-26 15:06         ` Andrey Konovalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1505937307.3026.20.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=andreyknvl@google.com \
    --cc=chunkeey@googlemail.com \
    --cc=dvyukov@google.com \
    --cc=kcc@google.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=syzkaller@googlegroups.com \
    --cc=tj@kernel.org \
    --cc=yong.zhang0@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.