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

[Paul E. McKenney]

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.

							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.
>