Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c3cf7c0c-27fe-4dfa-4249-a7e1747237a5@kernel.org>
Date:   Tue, 1 Mar 2022 19:44:38 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH] tracing/osnoise: Force quiescent states while tracing
Content-Language: en-US
To:     paulmck@kernel.org, Nicolas Saenz Julienne <nsaenzju@redhat.com>
Cc:     rostedt@goodmis.org, mingo@redhat.com,
        linux-kernel@vger.kernel.org, mtosatti@redhat.com
References: <20220228141423.259691-1-nsaenzju@redhat.com>
 <137d3573-051f-5374-70d6-cc99b44d00da@kernel.org>
 <b604526d3186f6cd3da189abb70bd1ad9a6105c5.camel@redhat.com>
 <69e92bc6-0917-17e9-1b61-d884d30ba42c@kernel.org>
 <20220301180509.GQ4285@paulmck-ThinkPad-P17-Gen-1>
From:   Daniel Bristot de Oliveira <bristot@kernel.org>
In-Reply-To: <20220301180509.GQ4285@paulmck-ThinkPad-P17-Gen-1>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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