Message-ID: <aXobzoeooJqxMkEj@hovoldconsulting.com>
Date: Wed, 28 Jan 2026 15:23:10 +0100
From: Johan Hovold <johan@...nel.org>
To: Tzung-Bi Shih <tzungbi@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...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>,
	Jason Gunthorpe <jgg@...dia.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 Tue, Jan 27, 2026 at 03:57:44PM +0000, Tzung-Bi Shih wrote:
> On Sat, Jan 24, 2026 at 06:05:32PM +0100, Johan Hovold wrote:
> > I was surprised to learn that the revocable functionality was merged last week
> > given the community feedback on list and at LPC, but not least since there are
> > no users of it, which we are supposed to require to be able to evaluate it
> > properly.
> > 
> > The chromeos ec driver issue which motivated this work turned out not to need
> > it as was found during review. And the example gpiolib conversion was posted

> Regarding cros_ec_chardev, my last attempt [2] to solve its hot-plug issue by
> synchronizing misc_{de,}register() with file operations using a global lock
> highlighted the difficulty of alternatives: that approach serialized all file
> operations and could easily lead to hung tasks if any file operation slept.

Yeah, fixing it at the chardev (or miscdevice) level would still require
some changes at the driver level (e.g. to wakeup sleeping tasks).

> Given the drawbacks of [2], I believe cros_ec_chardev remains a valid use
> case for revocable.

Yes, possibly. 

Miscdevice is a bit of a special case however since you cannot tie the
lifetime of your driver data to that of the miscdev, but there are ways
to address that. And some drivers already handle this (i.e. without any
changes to miscdevice).

But notably the revocable design seems to derive from this quirk of
miscdevice.

[ Side note for completeness: Looking at the cros_ec driver it doesn't
  seem to involve any hotpluggable buses so this looks like a mostly
  theoretical issue of a developer with root access actively unbinding
  an ec-driver while in use. ]

> > the very same morning that this was merged which hardly provides enough time
> > for evaluation (even if Bartosz quickly reported a performance regression).
> 
> The gpiolib conversion was provided as the first concrete user to enable
> this evaluation process.  The performance regression Bartosz identified is
> valuable feedback, and I believe it is addressed by [3].  I plan to send the
> next version of the series after v7.0-rc1 and revisit the issue.
> 
> [3] https://lore.kernel.org/all/20260121040204.2699886-1-tzungbi@kernel.org/
>
> > Turns out there are correctness issues with both the gpiolib conversion and
> > the revocable design itself that can lead to use-after-free and hung tasks (see
> > [1] and patch 3/3).
> 
> I appreciate you identifying the issues where multiple threads share a file
> descriptor; this is a case I overlooked.  I see these kinds of findings as a
> positive outcome of having wider review and a concrete user, allowing us to
> identify and fix issues in the design.

Yes, that's exactly why we always require a user *before* merging
something like this. To be able to determine that the interface is sane.

Now the user showed up on the same day as the merge, which is obviously
not enough time for proper review and discussion.

[ And by short-circuiting the normal process you probably reduce the
motivation for people to spend time on the example conversion too. ]

> > And as was pointed out repeatedly during review, and again at the day of the
> > merge, this does not look like the right interface for the chardev unplug
> > issue.
> 
> My focus has been on miscdevice [2], but I suspect cdev solutions for device
> hot-plug would face similar synchronization challenges between device removal
> and in-flight file operations.

But we need to look at that before throwing up our hands and saying that
it's not possible and that each driver should take care of this itself.

> > Revert the revocable implementation until a redesign has been proposed and
> > evaluated properly.
> 
> I'll work on addressing the discovered issues and send follow-up fixes.  I
> believe keeping the current series in linux-next would be beneficial, as it
> allows for easier testing and wider evaluation by others, rather than
> reverting at this stage.

To the contrary, now is right time to do the revert as there are
fundamental problems with the current interface that will require a
redesign. That's not the kind of work that should be done during an rc
stabilisation cycle.

Give people a chance to see for themselves what the gpiolib conversion
looks like, determine whether this abstraction makes the code more or
less readable, and think about possible further uses of a mechanism like
this.

Johan

> > [1] https://lore.kernel.org/all/aXT45B6vLf9R3Pbf@hovoldconsulting.com/

Powered by blists - more mailing lists