linux-kernel - Re: [PATCH v5 1/4] ftrace: Implement fs notification for tracing_max

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: <d8377cbd-8c3c-d699-153d-8af7b44d6b87@gmail.com>
Date:   Wed, 4 Sep 2019 21:00:32 +0200
From:   Viktor Rosendahl <viktor.rosendahl@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Joel Fernandes <joel@...lfernandes.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v5 1/4] ftrace: Implement fs notification for
 tracing_max_latency

On 9/4/19 1:39 PM, Steven Rostedt wrote:
> On Tue,  3 Sep 2019 15:25:59 +0200
> Viktor Rosendahl <viktor.rosendahl@...il.com> wrote:
> 
>> +void latency_fsnotify_stop(void)
>> +{
>> +	/* Make sure all CPUs see caller's previous actions to stop tracer */
>> +	smp_wmb();
> 
> These memory barriers just look wrong. What exactly are you trying to protect here?
> 
> Where's the matching rmbs?
> 

Thanks for reviewing.

However, since these functions will disappear when I take the irq_work
facility into use, we should perhaps not spend too much time discussing
what would have been.

There are no matching rmbs, I was thinking that the smp_wmb() would
merely enforce the order of the memory writes, as seen by other CPUs, so 
that the tracer would be stopped, before the latency fsnotify is disabled.

E.g. in case of the preemptirqsoff tracer the idea was that it doesn't
matter exactly when a CPU sees the "tracer_enabled = 0;" in
stop_irqsoff_tracer() but that it needs to be seen before the writes in
latency_fsnotify_stop() are seen.

best regards,

Viktor