[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y+KOeJlvQMYAaheZ@rowland.harvard.edu>
Date: Tue, 7 Feb 2023 12:46:32 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
LKML <linux-kernel@...r.kernel.org>,
USB list <linux-usb@...r.kernel.org>,
Hillf Danton <hdanton@...a.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: Converting dev->mutex into dev->spinlock ?
On Tue, Feb 07, 2023 at 10:07:18PM +0900, Tetsuo Handa wrote:
> On 2023/02/07 0:45, Alan Stern wrote:
> > On Mon, Feb 06, 2023 at 11:13:38PM +0900, Tetsuo Handa wrote:
> >> On 2023/02/05 10:23, Alan Stern wrote:
> >>> I suppose we could create separate lockdep classes for every bus_type
> >>> and device_type combination, as well as for the different sorts of
> >>> devices -- treat things like class devices separately from normal
> >>> devices, and so on. But even then there would be trouble.
> >>
> >> Sorry, since I'm not familiar with devices, I can't interpret what you
> >> are talking about in this response. But why don't you try test5() approach
> >> in an example module shown below (i.e. treat all dev->mutex instances
> >> independent to each other) ?
> >>
> >> Sharing mutex_init() (like test2() approach) causes false positives,
> >> but allocating a key on each dev->mutex (like test5() approach) should
> >> avoid false positives.
> >
> > Interesting idea. I'm doubtful that it will accomplish all that you
> > want. After all, one of lockdep's biggest advantages is that it can
> > detect the potential for deadlocks without a deadlock actually
> > occurring. By putting each mutex into its own class, you lose much of
> > this ability.
> >
> > But who knows? Maybe it will be a big help.
> >
> > Anyway, below is a patch you can try, based on the code for your test5.
> > Let me know what happens.
> >
>
> It boots, except lockdep_register_key() hit WARN_ON_ONCE() at
> device_register(&platform_bus) from platform_bus_init(), for
> platform_bus is a static object.
>
> struct device platform_bus = {
> .init_name = "platform",
> };
>
> We need to skip lockdep_register_key()/lockdep_unregister_key() on
> static "struct device" instances...
Okay, no doubt you can modify the patch to handle that.
The real question is what will happen in your syzbot test scenarios.
Lockdep certainly ought to be able to detect a real deadlock when one
occurs. It will be more interesting to find out if it can warn about
potential deadlocks _without_ them occurring.
Alan Stern
Powered by blists - more mailing lists