From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755717AbcC2E7S (ORCPT <rfc822;w@1wt.eu>);
	Tue, 29 Mar 2016 00:59:18 -0400
Received: from mail-pa0-f43.google.com ([209.85.220.43]:35471 "EHLO
	mail-pa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751224AbcC2E7P (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 29 Mar 2016 00:59:15 -0400
Date: Mon, 28 Mar 2016 21:59:09 -0700
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Wangnan (F)" <wangnan0@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
        Arnaldo Carvalho de Melo <acme@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>, linux-kernel@vger.kernel.org,
        Brendan Gregg <brendan.d.gregg@gmail.com>,
        He Kuang <hekuang@huawei.com>, Jiri Olsa <jolsa@kernel.org>,
        Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
        Namhyung Kim <namhyung@kernel.org>, pi3orama@163.com,
        Zefan Li <lizefan@huawei.com>
Subject: Re: [PATCH 4/4] perf core: Add backward attribute to perf event
Message-ID: <20160329045908.GB9017@ast-mbp.thefacebook.com>
References: <1459147292-239310-1-git-send-email-wangnan0@huawei.com>
 <1459147292-239310-5-git-send-email-wangnan0@huawei.com>
 <56F9E1F4.2050807@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <56F9E1F4.2050807@huawei.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Mar 29, 2016 at 10:01:24AM +0800, Wangnan (F) wrote:
> 
> 
> On 2016/3/28 14:41, Wang Nan wrote:
> 
> [SNIP]
> 
> >
> >To prevent this problem, we need to find a way to ensure the ring buffer
> >is stable during reading. ioctl(PERF_EVENT_IOC_PAUSE_OUTPUT) is
> >suggested because its overhead is lower than
> >ioctl(PERF_EVENT_IOC_ENABLE).
> >
> 
> Add comment:
> 
> By carefully verifying 'header' pointer, reader can avoid pausing the
> ring-buffer. For example:
> 
>     /* A union of all possible events */
>     union perf_event event;
> 
>     p = head = perf_mmap__read_head();
>     while (true) {
>         /* copy header of next event */
>         fetch(&event.header, p, sizeof(event.header));
> 
>         /* read 'head' pointer */
>         head = perf_mmap__read_head();
> 
>         /* check overwritten: is the header good? */
>         if (!verify(sizeof(event.header), p, head))
>             break;
> 
>         /* copy the whole event */
>         fetch(&event, p, event.header.size);
> 
>         /* read 'head' pointer again */
>         head = perf_mmap__read_head();
> 
>         /* is the whole event good? */
>         if (!verify(event.header.size, p, head))
>             break;
>         p += event.header.size;
>     }
> 
> However, the overhead is high because:
> 
>  a) In-place decoding is unsafe. Copy-verifying-decode is required.
>  b) Fetching 'head' pointer requires additional synchronization.

Such trick may work, but pause is needed for more than stability
of reading. When we collect the events into overwrite buffer
we're waiting for some other trigger (like all cpu utilization
spike or just one cpu running and all others are idle) and when
it happens the buffer has valuable info from the past. At this
point new events are no longer interesting and buffer should
be paused, events read and unpaused until next trigger comes.