Re: [PATCH v4 04/22] perf: Add a capability for AUX_NO_SG pmus to do software double buffering

From: Peter Zijlstra <peterz@infradead.org>
To: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org, Robert Richter <rric@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Stephane Eranian <eranian@google.com>,
	Andi Kleen <ak@linux.intel.com>,
	kan.liang@intel.com
Subject: Re: [PATCH v4 04/22] perf: Add a capability for AUX_NO_SG pmus to do software double buffering
Date: Mon, 8 Sep 2014 09:17:12 +0200	[thread overview]
Message-ID: <20140908071712.GT19379@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1408538179-792-5-git-send-email-alexander.shishkin@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

On Wed, Aug 20, 2014 at 03:36:01PM +0300, Alexander Shishkin wrote:
> For pmus that don't support scatter-gather for AUX data in hardware, it
> might still make sense to implement software double buffering to avoid
> losing data while the user is reading data out. For this purpose, add
> a pmu capability that guarantees multiple high-order chunks for AUX buffer,
> so that the pmu driver can do switchover tricks.

Please expand this with more detail on how to use this.

> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  include/linux/perf_event.h  |  1 +
>  kernel/events/ring_buffer.c | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index fe10bf6f94..1e7b659b49 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -172,6 +172,7 @@ struct perf_event;
>   */
>  #define PERF_PMU_CAP_NO_INTERRUPT		0x01
>  #define PERF_PMU_CAP_AUX_NO_SG			0x02
> +#define PERF_PMU_CAP_AUX_SW_DOUBLEBUF		0x04
>  
>  /**
>   * struct pmu - generic performance monitoring unit
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index d10919ca42..f5ee3669f8 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -286,9 +286,22 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
>  	if (!has_aux(event))
>  		return -ENOTSUPP;
>  
> -	if (event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG)
> +	if (event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) {
>  		order = get_order(nr_pages * PAGE_SIZE);
>  
> +		/*
> +		 * PMU requests more than one contiguous chunks of memory
> +		 * for SW double buffering
> +		 */
> +		if ((event->pmu->capabilities & PERF_PMU_CAP_AUX_SW_DOUBLEBUF) &&
> +		    !overwrite) {
> +			if (!order)
> +				return -EINVAL;
> +
> +			order--;
> +		}
> +	}

In particular this looks like it will allocate double the total amount
of pages and 'loose' half of them. There is no corresponding code in the
free path to collect them.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]