linux-kernel - Re: [PATCH] tracing: Remove pointless memory barriers

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: <20250626123445.5b01849d@gandalf.local.home>
Date: Thu, 26 Jun 2025 12:34:45 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Nam Cao <namcao@...utronix.de>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, linux-trace-kernel@...r.kernel.org,
 linux-kernel@...r.kernel.org, Gabriele Monaco <gmonaco@...hat.com>,
 john.ogness@...utronix.de
Subject: Re: [PATCH] tracing: Remove pointless memory barriers

On Thu, 26 Jun 2025 18:04:59 +0200
Nam Cao <namcao@...utronix.de> wrote:

> I think you have it inverted? I assume you meant:
> 
> "Without the barriers, the tr->buffer_disabled = 1 can be set on one CPU,
> and the other CPU can think the buffer is still enabled and do work that
> will end up doing nothing."
> 
> Your scenario can still happen despite the memory barrier:

Yes, but the point isn't really to prevent the race. It's more about making
the race window smaller.

When we disable it, if something is currently using it then it may or may
not get in. That's fine as this isn't critical.

But from my understanding, without the barriers, some architectures may
never see the update. That is, the write from one CPU may not get to memory
for a long time and new incoming readers will still see the old data. I'm
more concerned with new readers than ones that are currently racing with
the updates.

> 
> CPU1                          CPU2
>                               smp_rb()
>                               read buffer_disabled, see 0 --> let's do work!
> buffer_disabled=1
> smp_wb()
>                               do work -> end up doing nothing
> 
> >From my understanding, smp_wb()'s purpose is ensuring the ordering of one  
> write and another write, e.g.:
>     write(a)
>     smp_wb()
>     write(b)
> 
> For our case, there is only a single write. Therefore I don't think
> smp_wb() is useful.

Well, it does make it visible for other CPUs that do not have strong cache
coherency.

-- Steve