From: Johan Hovold <johan@kernel.org>
To: Bartosz Golaszewski <brgl@kernel.org>
Cc: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
Srinivas Kandagatla <srini@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/7] nvmem: survive unbind with active consumers
Date: Tue, 20 Jan 2026 11:18:30 +0100 [thread overview]
Message-ID: <aW9Wdpha1IB19-Rd@hovoldconsulting.com> (raw)
In-Reply-To: <CAMRc=Mf4P6g7fSU3YNFiZ1-TLTey4CeRJVsLjJ9JyJ8byDHTYg@mail.gmail.com>
On Mon, Jan 19, 2026 at 01:43:01PM +0100, Bartosz Golaszewski wrote:
> On Mon, Jan 19, 2026 at 12:22 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Fri, Jan 16, 2026 at 12:01:07PM +0100, Bartosz Golaszewski wrote:
> > > Nvmem is one of the subsystems vulnerable to object life-time issues.
> > > The memory nvmem core dereferences is owned by nvmem providers which can
> > > be unbound at any time and even though nvmem devices themselves are
> > > reference-counted, there's no synchronization with the provider modules.
> > >
> > > This typically is not a problem because thanks to fw_devlink, consumers
> > > get synchronously unbound before providers but it's enough to pass
> > > fw_devlink=off over the command line, unbind the nvmem controller with
> > > consumers still holding references to it and try to read/write in order
> > > to see fireworks in the kernel log.
> >
> > Well, don't do that then. Only root can unbind drivers, and we have
> > things like module references to prevent drivers from being unloaded
> > while in use.
>
> That's a very weird argument. It roughly translates to: "There's an
> issue that can crash the system but only root can trigger it so let's
> never fix it". If that's how we work then why don't we allow root to
> kill init with a signal?
It's not weird at all. The cost associated with preventing *root* from
using a foot gun may simply be too high for it to be worth it. For a
regular user, the equation is different.
> Is this a corner-case? Sure. Should we not address corner cases? Then
> why do we fix error paths we never hit or that result in an unusable
> system anyway?
That's just a false equivalence.
> Also: sysfs attributes don't take a module reference. Unloading an
> nvmem module during a long read from user-space is equivalent to a USB
> unplug.
Sysfs will drain any ongoing operations before deregistering, so not
sure what you're referring to here.
> > > User-space can trigger it too if a device (for instance: i2c eeprom on a
> > > cp2112 USB expander) is unplugged halfway through a long read.
> >
> > Hotplugging may be a real issue, though. But this can solved at the user
> > interface level. Did you explore that?
> >
>
> Did I explore what exactly? If you're saying this *can* be solved,
> then please also say how. Nvmem has no idea what abstraction layer it
> sits above. Nvmem core doesn't even know what abstraction layer the
> nvmem providers use.
The userspace interface for nvmem access. If the only interface is the
sysfs one then there should be nothing to worry about there.
> > For reference, this is related to the i2c discussion here:
> >
> > https://lore.kernel.org/lkml/aW4OWnyYp6Vas53L@hovoldconsulting.com/
> >
>
> Your argument against the i2c changes is that they affect lots of
> drivers and decrease readability. I can see it as a point worth
> considering. In the end it's up to Wolfram to decide and I think he
> was pretty clear he wants it to proceed.
>
> This series addresses the unbinding issue entirely within nvmem core,
> no driver changes are required. It uses SRCU which is very fast in
> read sections. What exactly is your argument against it? What is the
> reason for your comment? Unless you deal with nvmem internals, you
> never have to concern yourself with it.
My concern is that you're stuck on the idea of wrapping every access in
the kernel with unnecessary constructs because you seem to think driver
unbinding is something we need to worry about it. It's not. At least
not at any cost (readability, maintainability, cognitive load, churn,
performance, ...).
Johan
next prev parent reply other threads:[~2026-01-20 10:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-16 11:01 [PATCH 0/7] nvmem: survive unbind with active consumers Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 1/7] nvmem: remove unused field from struct nvmem_device Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 2/7] nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 3/7] nvmem: check the return value of gpiod_set_value_cansleep() Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 4/7] nvmem: simplify locking with guard() Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 5/7] nvmem: remove unneeded __nvmem_device_put() Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 6/7] nvmem: split struct nvmem_device into refcounted and provider-owned data Bartosz Golaszewski
2026-01-16 11:01 ` [PATCH 7/7] nvmem: synchronize nvmem device unregistering with SRCU Bartosz Golaszewski
2026-01-21 9:18 ` Bartosz Golaszewski
2026-01-19 11:22 ` [PATCH 0/7] nvmem: survive unbind with active consumers Johan Hovold
2026-01-19 12:43 ` Bartosz Golaszewski
2026-01-20 10:18 ` Johan Hovold [this message]
2026-01-20 19:57 ` Bartosz Golaszewski
2026-01-21 15:42 ` Johan Hovold
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=aW9Wdpha1IB19-Rd@hovoldconsulting.com \
--to=johan@kernel.org \
--cc=bartosz.golaszewski@oss.qualcomm.com \
--cc=brgl@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=srini@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