On Wed, Feb 11, 2026 at 02:05:26AM +0000, Tzung-Bi Shih wrote:
> On Mon, Feb 09, 2026 at 05:13:17AM -0600, Bartosz Golaszewski wrote:
> > On Fri, 6 Feb 2026 23:02:44 +0100, "Paul E. McKenney" <[email protected]>
> > said:
> > > On Fri, Feb 06, 2026 at 11:32:06AM +0100, Bartosz Golaszewski wrote:
> > >> Revocable stacks two layers of SRCU on top of each other: one to protect
> > >> the actual revocable resource and another to synchronize the revoking.
> > >> While this design itself is questionable, it also forces the user of
> > >> revokable to think about the implementation details and annotate the
> > >> pointer holding the address of the revocable_provider struct with __rcu.
> > >> Hide the real type of struct revokable_provider behind a typedef to free
> > >> the users from this responsability. While adding new typedefs goes
> > >> against current guidelines, it's still better than the current
> > >> requirement.
> > >>
> > >> Signed-off-by: Bartosz Golaszewski <[email protected]>
> > >> ---
> > >> I realized that one important person was missing from the whole review
> > >> process: Paul E. McKenney who wrote and maintains SRCU. I had Paul look
> > >> at the SRCU usage in GPIO and I think he should have also signed off on
> > >> revocable before it got queued.
> > >>
> > >> Paul: I'm Cc'ing you on this patch to bring revocable to your attention.
> > >> The series that implemented it and made its way into v7.0 is here:
> > >>
> > >> https://lore.kernel.org/all/[email protected]/
> > >>
> > >> Could you please take a look and say if the design looks sane to you?
> > >> Especially the double SRCU on the revocable_provider.
> > >
> > > The first patch in the above URL adds SRCU, and the other
> > > two add various tests. I do not see a double SRCU, just an
> > > srcu_read_lock() in revocable_try_access() and an srcu_read_unlock()
> > > in revocable_withdraw_access().
> > >
> > > You are allowed to nest srcu_read_lock(), if that is what you are asking.
> > > *However*, nesting revocable_try_access() on the same revocable structure
> > > is buggy because the second call to revocable_try_access() would overwrite
> > > the rp->srcu value written by the first call. This could result in both
> > > SRCU grace-period hangs and too-short SRCU grace periods, more likely
> > > the former than the latter.
> > >
> > > Or do you mean something else by "double SRCU"?
> > >
> >
> > This series didn't have it yet, it appeared as a fix to a race reported
> > after
> > it was queued, sorry for the confusion. I'm talking about this bit[1] here.
> > It returns an __rcu-annotated pointer, forcing the user to keep and manage
> > it.
> >
> > This is because when revoking the resource[2], the pointer storing the
> > address
> > of the revokable provider is also managed by SRCU - in addition to the
> > revokable resource itself which seems to me like a weird concept. I
> > understand
> > the race condition it fixes but I assumed the whole concept of revokable is
> > to
> > free the user from being bothered by the implementation details behind it
> > which
> > leak out of the API if you need to keep __rcu around.
>
> Please hold off on reviewing the patch and the "double SRCU" usage for now.
> I'll remove the second RCU in the next version, which should serve as a
> better starting point for a clean review.
Will do, thank you!
> Side note: It was actually one SRCU and one RCU, not a double SRCU.
That does sound more likely, though you really could nest SRCU or use
different srcu_struct structures in the same algorithm. But let's see
what you come up with.
Thanx, Paul
