public inbox for linux-tegra@vger.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: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260113162843.12712-1-hanguidong02@gmail.com>
     [not found] ` <7ae38e31-ef31-43ad-9106-7c76ea0e8596@sirena.org.uk>
     [not found]   ` <CGME20260120152328eucas1p1024a7488ae10b8b7f2fcb74baee24c75@eucas1p1.samsung.com>
     [not found]     ` <ef37ed64-24ad-4b82-bc6c-d3bc72aaf232@samsung.com>
2026-01-21 20:00       ` [PATCH v5] driver core: enforce device_lock for driver_match_device() 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox