Re: [PATCH v8 1/4] rust: helpers: add SRCU helpers

[Onur Özkan]

On Sat, 30 May 2026 12:01:21 -0700
"Paul E. McKenney" <paulmck@xxxxxxxxxx> wrote:

> On Fri, May 29, 2026 at 01:16:17PM +0100, Gary Guo wrote:
> > On Fri May 29, 2026 at 1:07 PM BST, Onur Özkan wrote:
> > > On Fri, 29 May 2026 12:58:17 +0100
> > > Gary Guo <gary@xxxxxxxxxxx> wrote:
> > >
> > >> On Fri May 29, 2026 at 12:43 PM BST, Onur Özkan wrote:
> > >> > Add helper wrappers for SRCU functions that are exposed to Rust
> > >> > through generated bindings.
> > >> >
> > >> > Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > >> > Signed-off-by: Onur Özkan <work@xxxxxxxxxxxxx>
> > >> > ---
> > >> >  include/linux/srcu.h   |  5 +++++
> > >> >  rust/helpers/helpers.c |  1 +
> > >> >  rust/helpers/srcu.c    | 30 ++++++++++++++++++++++++++++++
> > >> >  3 files changed, 36 insertions(+)
> > >> >  create mode 100644 rust/helpers/srcu.c
> > >> >
> > >> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > >> > index 81b1938512d5..790a4ef713c0 100644
> > >> > --- a/include/linux/srcu.h
> > >> > +++ b/include/linux/srcu.h
> > >> > @@ -57,6 +57,11 @@ int __init_srcu_struct_fast_updown(struct srcu_struct *ssp, const char *name,
> > >> >  #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > >> >  
> > >> >  int init_srcu_struct(struct srcu_struct *ssp);
> > >> > +static inline int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
> > >> > +				     struct lock_class_key *key)
> > >> > +{
> > >> > +	return init_srcu_struct(ssp);
> > >> > +}
> > >> 
> > >> 
> > >> This looks like a different strategy than, say, how mutex handles lockdep. IMO
> > >> the way that mutex handles this is more clear.
> > >
> > > Initially it was the same strategy, this one was suggested by Paul [1] and it
> > > didn't sound like a bad idea to me. Also, in previous revisions, Boqun once
> > > said:
> > 
> > I'm talking about changing how this is handled in SRCU side, not helper side.
> > 
> > >
> > > 	"Having a "#ifdef" in a function body is generally considered as bad code
> > > 	(I know we have some of these in rust helpers, but helpers are a bit
> > > 	special and maybe we should clean them up). So could you move this
> > > 	function into include/linux/srcutiny.h and include/linux/srcutree.h?"
> > 
> > mutex.h doesn't use ifdef inside function body, it just define two static inline
> > functions with the same signature in different ifdef branches.
> 
> True, but SRCU has the added complication of having different !SMP (Tiny)
> and SMP (Tree) implementations.  On the other hand, it is quite possible
> that I missed a trick when setting up this part of SRCU.  Please feel
> free to send a patch that simplifies it.

It's on the list [1].

Thanks,
Onur

[1]: https://lore.kernel.org/all/20260529134004.396743-2-work@xxxxxxxxxxxxx

> 
> 							Thanx, Paul
> 
> > Best,
> > Gary
> > 
> > >
> > > So putting them together, I decided to follow Paul's suggestion.
> > >
> > > [1]: https://lore.kernel.org/all/05906890-cf06-414c-a625-02f9d7150d57@paulmck-laptop
> > >
> > >> 
> > >> If we follow the way mutex is handles this, the `init_srcu_struct` would always
> > >> create lock class key and forward to `__init_srcu_struct`, and
> > >> `__init_srcu_struct` is an inline function that either calls
> > >> `init_srcu_struct_lockdep` with all args, or discard name/key and forward to
> > >> `init_srcu_struct_mutex`.
> > >> 
> > >> This is beneficial because the usage flows one way. You don't have in one config
> > >> `init_srcu_struct` call `__init_srcu_struct` and have the opposite in another
> > >> config, which I think can be quite confusing.
> >