linux-kernel - Re: [PATCH] tracing/osnoise: Force quiescent states while tracing

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: <c3cf7c0c-27fe-4dfa-4249-a7e1747237a5@kernel.org>
Date:   Tue, 1 Mar 2022 19:44:38 +0100
From:   Daniel Bristot de Oliveira <bristot@...nel.org>
To:     paulmck@...nel.org, Nicolas Saenz Julienne <nsaenzju@...hat.com>
Cc:     rostedt@...dmis.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, mtosatti@...hat.com
Subject: Re: [PATCH] tracing/osnoise: Force quiescent states while tracing

On 3/1/22 19:05, Paul E. McKenney wrote:
>> I see, as long as it costs < 1 us, I am ok. If it gets > 1us in a reasonably
>> fast machine, we start see HW noise where it does not exist, and that would
>> reduce the resolution of osnoise. AFAICS, it is not causing that problem, but we
>> need to make it as lightweight as possible.
> In the common case, it is atomically incrementing a local per-CPU counter
> and doing a store.  This should be quite cheap.
> 
> The uncommon case is when the osnoise process was preempted or otherwise
> interfered with during a recent RCU read-side critical section and
> preemption was disabled around that critical section's outermost
> rcu_read_unlock().  This can be quite expensive.  But I would expect
> you to just not do this.  ;-)

Getting the expensive call after a preemption is not a problem, it is a side
effect of the most costly preemption.

It this case, we should "ping rcu" before reading the time to account the
overhead for the previous preemption which caused it.

like (using the current code as example):

------------------------- %< -------------------------------
static u64
set_int_safe_time(struct osnoise_variables *osn_var, u64 *time)
{
        u64 int_counter;

        do {
                int_counter = local_read(&osn_var->int_counter);

		------------> HERE <-------------------------------------

                /* synchronize with interrupts */
                barrier();

                *time = time_get();

                /* synchronize with interrupts */
                barrier();
        } while (int_counter != local_read(&osn_var->int_counter));

        return int_counter;
}
------------------------- >% -------------------------------

In this way anything that happens before this *time is accounted before it is
get. If anything happens while this loop is running, it will run again, so it is
safe to point to the previous case.

We would have to make a copy of this function, and only use the copy for the
run_osnoise() case. A good name would be something in the lines of
set_int_safe_time_rcu().

(Unless the expensive is < than 1us.)

-- Daniel