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

[Gary Guo]

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.

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.