<!DOCTYPE html>
<!-- BaNnErBlUrFlE-BoDy-start -->
<!-- Preheader Text : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;">
On Thu, Jul 30, 2026 at 6: 02 PM Paolo Abeni <pabeni@ redhat. com> 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</div>
<!-- Preheader Text : END -->
<!-- Email Banner : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;"></div>
<!-- Email Banner : END -->
<!-- BaNnErBlUrFlE-BoDy-end -->
<html>
<head><!-- BaNnErBlUrFlE-HeAdEr-start -->
<style>
#pfptBannerj8uah6e { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important;
background-color: #c2d4d4 !important;
max-width: none !important; max-height: none !important }
.pfptPrimaryButtonj8uah6e:hover, .pfptPrimaryButtonj8uah6e:focus {
background-color: #a2b1b1 !important; }
.pfptPrimaryButtonj8uah6e:active {
background-color: #828e8e !important; }
html:root, html:root>body { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important; }
</style>
<!-- BaNnErBlUrFlE-HeAdEr-end -->
</head><body><pre style="font-family: sans-serif; font-size: 100%; white-space: pre-wrap; word-wrap: break-word">On Thu, Jul 30, 2026 at 6:02 PM Paolo Abeni <pabeni@redhat.com> 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
</pre></body></html>