linux-kernel - Re: [PATCH v4 02/22] perf: Add AUX area to ring buffer for raw data streams

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: <87lhpuuztj.fsf@ashishki-desk.ger.corp.intel.com>
Date:	Mon, 08 Sep 2014 14:16:56 +0300
From:	Alexander Shishkin <alexander.shishkin@...ux.intel.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Robert Richter <rric@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mike Galbraith <efault@....de>,
	Paul Mackerras <paulus@...ba.org>,
	Stephane Eranian <eranian@...gle.com>,
	Andi Kleen <ak@...ux.intel.com>, kan.liang@...el.com
Subject: Re: [PATCH v4 02/22] perf: Add AUX area to ring buffer for raw data streams

Peter Zijlstra <peterz@...radead.org> writes:

> On Wed, Aug 20, 2014 at 03:35:59PM +0300, Alexander Shishkin wrote:
>> From: Peter Zijlstra <peterz@...radead.org>
>> 
>> This patch introduces "AUX space" in the perf mmap buffer, intended for
>> exporting high bandwidth data streams to userspace, such as instruction
>> flow traces.
>> 
>> AUX space is a ring buffer, defined by aux_{offset,size} fields in the
>> user_page structure, and read/write pointers aux_{head,tail}, which abide
>> by the same rules as data_* counterparts of the main perf buffer.
>> 
>> In order to allocate/mmap AUX, userspace needs to set up aux_offset to
>> such an offset that will be greater than data_offset+data_size and
>> aux_size to be the desired buffer size. Both need to be page aligned.
>> Then, same aux_offset and aux_size should be passed to mmap() call and
>> if everything adds up, you should have an AUX buffer as a result.
>> 
>> Pages that are mapped into this buffer also come out of user's mlock
>> rlimit plus perf_event_mlock_kb allowance.
>> 
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
>> ---
>
>> +void rb_free_aux(struct ring_buffer *rb, struct perf_event *event)
>> +{
>> +	struct perf_event *iter;
>> +	int pg;
>> +
>> +	if (rb->aux_priv) {
>> +		/* disable all potential writers before freeing */
>> +		rcu_read_lock();
>> +		list_for_each_entry_rcu(iter, &rb->event_list, rb_entry)
>> +			perf_event_disable(iter);
>> +		rcu_read_unlock();
>
> Hmm, I cannot remember this from the last time; and its not explained
> why this was added.
>
> This would change semantics between munmap of a buffer with and without
> AUX bits in. 

Indeed, but this seems fair: if an event is generating AUX data while we
unmap the AUX buffer, there is no reason to keep it on, because chances
are, AUX data is the only thing this event is generating. The
alternative would be to have a separate callback for this, but then
again, that would race with other pmu callbacks, since it would be
called from munmap() path. I should add an elaborate comment about this.

Does this sound reasonable?

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/