[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260126000730.GI1134360@nvidia.com>
Date: Sun, 25 Jan 2026 20:07:30 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Johan Hovold <johan@...nel.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Tzung-Bi Shih <tzungbi@...nel.org>,
Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>,
Linus Walleij <linusw@...nel.org>, Jonathan Corbet <corbet@....net>,
Shuah Khan <shuah@...nel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Simona Vetter <simona.vetter@...ll.ch>,
Dan Williams <dan.j.williams@...el.com>, linux-doc@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] Revert "revocable: Revocable resource management"
On Sun, Jan 25, 2026 at 06:53:15PM +0100, Danilo Krummrich wrote:
>
> Let's take some DRM IOCTL for instance:
>
> struct MyDriver {
> bar: Devres<pci::Bar>,
> }
>
> fn ioctl_vm_create(
> drm: &drm::Device<MyDriver>,
> req: &mut uapi::drm_mydriver_vm_create,
> file: &drm::File<MyDriverFile>,
> ) -> Result<u32> {
> // Runs the closure in an (S)RCU read side critical section if the
> // resource is available, returns ENXIO otherwise.
> drm.bar.try_access_with(|bar| {
> // (S)RCU read side critical section starts here.
>
> bar.write32(...);
>
> // (S)RCU read side critical section ends here.
> }).ok_or(ENXIO)?;
>
> Ok(0)
> }
That's the whole issue with DRM right there - allowing driver code to
run after the driver has unregistered from the subsystem is
*dangerous* and creates all these bugs.
>From a rust perspective I would argue you should be looking at every
one of those try_access_with() sites in drivers as a code smell that
says something is questionable in the driver or subsystem.
In many other subsystems a driver should *never* use
"try_access_with". Unfortunately the rust solution forces
synchronize_srcu()'s anyway (which Bartoz rightly pointed out as an
unacceptable performance issue). IMHO since rust has the Device<Bound>
stuff the revocable should have used rwsem, because the expectation
should be that the majority uses access, not try_access.
> I think those examples make it obvious why a revocable implementation on the C
> side can't provide the same value and ergonomics due to language limitations,
> yet I think it makes sense to start experimenting how subsystems can adopt this
> design-pattern in C.
The most important part of this pattern, IMHO, is documenting when you
are in a safe Device<Bound> scope or not.
What the C version of revocable does is just enforce it *everwhere*
without any thought as to if it is papering over a bigger problem.
In C protecting some limited "device resources" is not nearly good
enough for alot of drivers since the root issue here is often the
author doesn't understand the undocumented contexts when it is
"try_access_with" vs "access" and then makes a lot more errors than
just "device resources". :(
Frankly I don't think "iterating" is going to salvage this idea. The
real value from rust was not in creating a thin wrapper around
SRCU. It is in all the other stuff Danilo has explained.
Jason
Powered by blists - more mailing lists