All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Gui-Dong Han" <hanguidong02@gmail.com>
Cc: "Jon Hunter" <jonathanh@nvidia.com>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mark Brown" <broonie@kernel.org>, <gregkh@linuxfoundation.org>,
	<rafael@kernel.org>, <linux-kernel@vger.kernel.org>,
	<baijiaju1990@gmail.com>, "Qiu-ji Chen" <chenqiuji666@gmail.com>,
	<Aishwarya.TCV@arm.com>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()
Date: Fri, 23 Jan 2026 20:07:44 +0100	[thread overview]
Message-ID: <DFW7DOC56CUG.3PV4UGDTMUYE1@kernel.org> (raw)
In-Reply-To: <CALbr=LZ4v7N=tO1vgOsyj9AS+XuNbn6kG-QcF+PacdMjSo0iyw@mail.gmail.com>

On Fri Jan 23, 2026 at 7:53 PM CET, Gui-Dong Han wrote:
> It seems the issue is simpler than a recursive registration deadlock.
> Looking at the logs, tegra_qspi_probe triggers a NULL pointer
> dereference (Oops) while holding the device_lock. The mutex likely
> remains marked as held/orphaned, blocking subsequent driver bindings
> on the same bus.
>
> This likely explains why lockdep was silent. Since this is not a lock
> dependency cycle or a recursive locking violation, but rather a lock
> remaining held by a terminated task, lockdep would not flag it as a
> deadlock pattern.
>
> This is indeed a side effect of enforcing the lock here—it amplifies
> the impact of a crash. However, an Oops while holding the device_lock
> is generally catastrophic regardless.

This makes sense to me; it might indeed be as simple as that.

> Following up on our previous discussion [1], refactoring
> driver_override would resolve this. We could move driver_override to
> struct device and protect it with a dedicated lock (e.g.,
> driver_override_lock). We would then replace driver_set_override with
> dev_set_driver_override and add dev_access_driver_override with
> internal lock assertions. This allows us to remove device_lock from
> the 2 match paths, reducing contention and preventing a single crash
> from stalling the whole bus.
>
> However, this deviates from the current paradigm where device_lock
> protects sysfs attributes (like waiting_for_supplier and
> power/control). If other sysfs attributes are found to share similar
> constraints or would benefit from finer-grained locking (which
> requires further investigation), we might have a stronger argument for
> introducing a more generic sysfs_lock to handle this class of
> attributes. We would also need to carefully verify safety during
> device removal.
>
> Danilo, what are your thoughts on this refactoring plan? I am willing
> to attempt it, but since it touches the driver core, documentation,
> and 10+ bus drivers, and I haven't submitted such a large series
> before, it may take me a few weeks to get an initial version out, and
> additional time to iterate based on review feedback until it is ready
> for merging. If you prefer to handle it yourself to expedite things,
> please let me know so we don't duplicate efforts.

I think moving driver_override to struct device and providing accessors with
proper lockdep assertions is the correct thing to do. With that, I do not think
a separate lock is necessary.

Please feel free to follow up on this.

  reply	other threads:[~2026-01-23 19:07 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13 16:28 [PATCH v5] driver core: enforce device_lock for driver_match_device() Gui-Dong Han
2026-01-13 16:35 ` Rafael J. Wysocki
2026-01-13 19:23 ` Danilo Krummrich
2026-01-16  7:34   ` Gui-Dong Han
2026-01-16 11:19     ` Greg KH
2026-01-16 11:38       ` Gui-Dong Han
2026-01-16 11:54 ` Danilo Krummrich
2026-01-20 13:22 ` Mark Brown
2026-01-20 13:30   ` Gui-Dong Han
2026-01-20 13:48     ` Mark Brown
2026-01-20 14:05       ` Gui-Dong Han
2026-01-21  8:55     ` Wang Jiayue
2026-01-21  8:57       ` Gui-Dong Han
2026-01-21 10:40       ` Danilo Krummrich
2026-01-21 11:02         ` Danilo Krummrich
2026-01-21 11:19           ` Greg KH
2026-01-21 12:49           ` Mark Brown
2026-01-21 12:50             ` Danilo Krummrich
2026-01-21 13:02               ` Will Deacon
2026-01-21 14:07                 ` Danilo Krummrich
2026-01-21 13:03           ` Robin Murphy
2026-01-21 14:13             ` Danilo Krummrich
2026-01-21 13:22           ` Jiayue Wang
2026-01-20 15:03   ` Danilo Krummrich
2026-01-20 15:35     ` Mark Brown
2026-01-20 17:38     ` Mark Brown
2026-01-20 18:36       ` Danilo Krummrich
2026-01-20 20:05         ` Mark Brown
2026-01-20 21:18           ` Danilo Krummrich
2026-01-21  1:11             ` Danilo Krummrich
2026-01-21  7:18               ` Gui-Dong Han
2026-01-21  7:41                 ` Gui-Dong Han
2026-01-21  7:56                   ` Greg KH
2026-01-21  8:12                     ` Greg KH
2026-01-21  9:54                     ` Danilo Krummrich
2026-01-21 10:30                       ` Greg KH
2026-01-20 15:23   ` Marek Szyprowski
2026-01-20 15:27     ` Mark Brown
2026-01-21 20:00     ` Jon Hunter
2026-01-21 21:42       ` Danilo Krummrich
2026-01-22 17:28         ` Jon Hunter
2026-01-22 17:55           ` Gui-Dong Han
2026-01-22 18:12             ` Danilo Krummrich
2026-01-22 18:58               ` Jon Hunter
2026-01-22 19:35                 ` Danilo Krummrich
2026-01-23 13:57                   ` Jon Hunter
2026-01-23 14:09                     ` Danilo Krummrich
2026-01-23 14:29                       ` Jon Hunter
2026-01-23 16:54                         ` Danilo Krummrich
2026-01-23 18:53                           ` Gui-Dong Han
2026-01-23 19:07                             ` Danilo Krummrich [this message]
2026-01-27 14:58                               ` Jon Hunter
2026-01-27 15:18                                 ` Danilo Krummrich
2026-01-27 14:53                   ` Jon Hunter
2026-01-27 15:05                     ` Gui-Dong Han
2026-01-21  7:40   ` David Heidelberg
2026-02-11 10:42   ` Alexander Stein
2026-02-11 13:56     ` Danilo Krummrich
2026-02-25 20:19 ` Cristian Marussi
2026-02-25 20:38   ` Danilo Krummrich
2026-02-26  8:54     ` Gatien CHEVALLIER
2026-02-26 11:15       ` Danilo Krummrich
2026-02-26 12:21         ` Cristian Marussi

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=DFW7DOC56CUG.3PV4UGDTMUYE1@kernel.org \
    --to=dakr@kernel.org \
    --cc=Aishwarya.TCV@arm.com \
    --cc=baijiaju1990@gmail.com \
    --cc=broonie@kernel.org \
    --cc=chenqiuji666@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hanguidong02@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=rafael@kernel.org \
    /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.