linux-kernel - Re: [RFC 2/3] preempt_tracer: Disable IRQ while starting/stopping due to a preempt

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]

Date:   Wed, 29 May 2019 11:40:34 +0200
From:   Daniel Bristot de Oliveira <bristot@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, williams@...hat.com,
        daniel@...stot.me, "Steven Rostedt (VMware)" <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        "Joel Fernandes (Google)" <joel@...lfernandes.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Yangtao Li <tiny.windzz@...il.com>,
        Tommaso Cucinotta <tommaso.cucinotta@...tannapisa.it>
Subject: Re: [RFC 2/3] preempt_tracer: Disable IRQ while starting/stopping due
 to a preempt_counter change

On 29/05/2019 10:33, Peter Zijlstra wrote:
> On Tue, May 28, 2019 at 05:16:23PM +0200, Daniel Bristot de Oliveira wrote:
>> The preempt_disable/enable tracepoint only traces in the disable <-> enable
>> case, which is correct. But think about this case:
>>
>> ---------------------------- %< ------------------------------
>> 	THREAD					IRQ
>> 	   |					 |
>> preempt_disable() {
>>     __preempt_count_add(1)
>> 	------->	    smp_apic_timer_interrupt() {
>> 				preempt_disable()
>> 				    do not trace (preempt count >= 1)
>> 				    ....
>> 				preempt_enable()
>> 				    do not trace (preempt count >= 1)
>> 			    }
>>     trace_preempt_disable();
>> }
>> ---------------------------- >% ------------------------------
>>
>> The tracepoint will be skipped.
> 
> .... for the IRQ. But IRQs are not preemptible anyway, so what the
> problem?


right, they are.

exposing my problem in a more specific way:

To show in a model that an event always takes place with preemption disabled,
but not necessarily with IRQs disabled, it is worth having the preemption
disable events separated from IRQ disable ones.

The main reason is that, although IRQs disabled postpone the execution of the
scheduler, it is more pessimistic, as it also delays IRQs. So the more precise
the model is, the less pessimistic the analysis will be.

But there are other use-cases, for instance:

(Steve, correct me if I am wrong)

The preempt_tracer will not notice a "preempt disabled" section in an IRQ
handler if the problem above happens.

(Yeah, I know these problems are very specific... but...)

>> To avoid skipping the trace, the change in the counter should be "atomic"
>> with the start/stop, w.r.t the interrupts.
>>
>> Disable interrupts while the adding/starting stopping/subtracting.
> 
>> +static inline void preempt_add_start_latency(int val)
>> +{
>> +	unsigned long flags;
>> +
>> +	raw_local_irq_save(flags);
>> +	__preempt_count_add(val);
>> +	preempt_latency_start(val);
>> +	raw_local_irq_restore(flags);
>> +}
> 
>> +static inline void preempt_sub_stop_latency(int val)
>> +{
>> +	unsigned long flags;
>> +
>> +	raw_local_irq_save(flags);
>> +	preempt_latency_stop(val);
>> +	__preempt_count_sub(val);
>> +	raw_local_irq_restore(flags);
>> +}
> 
> That is hideously expensive :/

Yeah... :-( Is there another way to provide such "atomicity"?

Can I use the argument "if one has these tracepoints enabled, they are not
considering it as a hot-path?"

-- Daniel