Re: [PATCH net-next v2] rds: synchronize info callbacks with module unload

[Chengfeng Ye]

On Thu, Jul 30, 2026 at 6:02 PM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
>
> Hi,
>
> It looks like I was too optimistic in my previous reply.
>
> On 7/27/26 7:48 PM, Chengfeng Ye wrote:
> > @@ -78,8 +80,13 @@ void rds_info_register_func(int optname, rds_info_func func)
> >       BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST);
> >
> >       spin_lock(&rds_info_lock);
> > -     BUG_ON(rds_info_funcs[offset]);
> > -     rds_info_funcs[offset] = func;
> > +     /* Validate the latest published slot value. */
> > +     if (WARN_ON_ONCE(smp_load_acquire(&rds_info_funcs[offset]))) {
>
> This read is under the same lock that perform the store. It's totally
> unclear why smp_load_acquire() would be needed here. Very reasonably
> it's not.
>
> > +             spin_unlock(&rds_info_lock);
> > +             return;
> > +     }
> > +     /* Pair with lockless callback lookup. */
> > +     smp_store_release(&rds_info_funcs[offset], func);
> >       spin_unlock(&rds_info_lock);
> >  }
> >  EXPORT_SYMBOL_GPL(rds_info_register_func);
> > @@ -91,9 +98,15 @@ void rds_info_deregister_func(int optname, rds_info_func func)
> >       BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST);
> >
> >       spin_lock(&rds_info_lock);
> > -     BUG_ON(rds_info_funcs[offset] != func);
> > -     rds_info_funcs[offset] = NULL;
> > +     /* Validate the latest published slot value. */
> > +     if (WARN_ON_ONCE(smp_load_acquire(&rds_info_funcs[offset]) != func)) {
>
> Same here.
>
> /P
>

Hi Paolo,

You are right, that two smp_load_acquire() are unnecessary. Should I
send a v3 patch to change that two smp_load_acquire() under locked to
use back plain read?

Thanks, Chengfeng