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: <01576402efe6a5a76d895eca367aa01e7f169d3d.camel@oracle.com>
Date: Fri, 7 Mar 2025 20:28:57 +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 Thu, 2025-03-06 at 10:18 -0800, Jakub Kicinski wrote:
> On Thu, 6 Mar 2025 16:41:35 +0000 Allison Henderson wrote:
> > 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).
> 
> Let's be precise, can you give an example of 2 execution threads
> and memory accesses which have to be ordered.

Hi Jakub,

I just realized my last response referred to bits and functions in the next patch instead this of one.  Apologies for
the confusion!  For this thread example though, I think a pair of threads in rds_send_worker and rds_sendmsg would be a
good example?  How about this:

Thread A:
  Calls rds_send_worker()
    calls rds_clear_queued_send_work_bit()
      clears RDS_SEND_WORK_QUEUED in cp->cp_flags
    calls rds_send_xmit()
    calls cond_resched()

Thread B:
   Calls rds_sendmsg()
   Calls rds_send_xmit
   Calls rds_cond_queue_send_work 
      checks and sets RDS_SEND_WORK_QUEUED in cp->cp_flags

So in this example the memory barriers ensure that the clearing of the bit is properly seen by thread B.  Without these
memory barriers in rds_clear_queued_send_work_bit(), rds_cond_queue_send_work() could see stale values in cp->cp_flags
and incorrectly assume that the work is still queued, leading to potential missed work processing.

I hope that helps some?  Let me know if so/not or if there is anything else that would help clarify.  If it helps at
all, I think there's a similar use case in commit 93093ea1f059, though it's the other way around with the barriers
around the set_bit, and the implicit barriers in the test_and_clear_bit().  And I think on CPUs with strongly ordered
memory, the barriers do not expand to anything in that case.

Let me know if this helps!
Thank you!

Allison