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: <2a426aa2-42c8-e839-1cec-aa3971651f3e@linux.intel.com>
Date:   Wed, 23 Aug 2017 11:54:15 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        Dmitri Prokhorov <Dmitry.Prohorov@...el.com>,
        Valery Cherepennikov <valery.cherepennikov@...el.com>,
        Mark Rutland <mark.rutland@....com>,
        Stephane Eranian <eranian@...gle.com>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 2/3]: perf/core: use context tstamp_data for skipped
 events on mux interrupt

On 22.08.2017 23:47, Peter Zijlstra wrote:
> On Thu, Aug 10, 2017 at 06:57:43PM +0300, Alexey Budankov wrote:
>> The key thing in the patch is explicit updating of tstamp fields for
>> INACTIVE events in update_event_times().
> 
>> @@ -1405,6 +1426,9 @@ static void update_event_times(struct perf_event *event)
>>  	    event->group_leader->state < PERF_EVENT_STATE_INACTIVE)
>>  		return;
>>  
>> +	if (event->state == PERF_EVENT_STATE_INACTIVE)
>> +		perf_event_tstamp_update(event);
>> +
>>  	/*
>>  	 * in cgroup mode, time_enabled represents
>>  	 * the time the event was enabled AND active
> 
> But why!? I thought the whole point was to not need to do this.

update_event_times() is not called from timer interrupt handler 
thus it is not on the critical path which is optimized in this patch set.

But update_event_times() is called in the context of read() syscall so
this is the place where we may update event times for INACTIVE events 
instead of timer interrupt.

Also update_event_times() is called on thread context switch out so
we get event times also updated when the thread migrates to other CPU.

> 
> The thing I outlined earlier would only need to update timestamps when
> events change state and at no other point in time.

But we still may request times while event is in INACTIVE state 
thru read() syscall and event timings need to be up-to-date. 

> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ