netdev - Re: [PATCH 1/6] net/rds: Avoid queuing superfluous send and recv work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ba0b69633769cd45fccf5715b9be9d869bc802ae.camel@oracle.com>
Date: Thu, 6 Mar 2025 16:41:35 +0000
From: Allison Henderson <allison.henderson@...cle.com>
To: "kuba@...nel.org" <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/6] net/rds: Avoid queuing superfluous send and recv work

On Tue, 2025-03-04 at 16:44 -0800, Jakub Kicinski wrote:
> On Wed, 5 Mar 2025 00:38:41 +0000 Allison Henderson wrote:
> > > I'm guessing the comments were added because checkpatch asked for them.
> > > The comments are supposed to indicate what this barrier pairs with.
> > > I don't see the purpose of these barriers, please document..  
> > 
> > Hi Jakob,
> > 
> > I think the comments meant to refer to the implicit memory barrier in
> > "test_and_set_bit".  It looks like it has assembly code to set the
> > barrier if CONFIG_SMP is set.  How about we change the comments to:
> > "pairs with implicit memory barrier in test_and_set_bit()" ?  Let me
> > know what you think.
> 
> Okay, but what is the purpose. The commit message does not explain 
> at all why these barriers are needed.

Hi Jakub,

I think it's to make sure the clearing of the bit is the last operation done for the calling function, in this case
rds_queue_reconnect.  The purpose of the barrier in test_and_set is to make sure the bit is checked before proceeding to
any further operations (in our case queuing reconnect items).  So it would make sense that the clearing of the bit
should happen only after we are done with all such operations.  I found some documentation for smp_mb__*_atomic in
Documentation/memory-barriers.txt that mentions these functions are used for atomic RMW bitop functions like clear_bit
and set_bit, since they do not imply a memory barrier themselves.  Perhaps the original comment meant to reference that
too.

I hope that helps?  If so, maybe we can expand the comment further to something like:
/*
 * pairs with implicit memory barrier in calls to test_and_set_bit with RDS_RECONNECT_PENDING
 * Used to ensure the bit is on only while reconnect operations are in progress
 */

 Let me know what you think, or that's too much or too little detail.

 Thanks!
 Allison