All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Wangnan (F)" <wangnan0@huawei.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.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
Date: Tue, 29 Mar 2016 13:59:57 +0800	[thread overview]
Message-ID: <56FA19DD.6040708@huawei.com> (raw)
In-Reply-To: <20160329045908.GB9017@ast-mbp.thefacebook.com>



On 2016/3/29 12:59, Alexei Starovoitov wrote:
> 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.

Agree. I just want to provide an alternative method.
I'm trying to finger out pausing is not mandatory
but highly recommended in man page and commit
messages.

Thank you.

  reply	other threads:[~2016-03-29  6:01 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-28  6:41 [PATCH 0/4] perf core: Support reading from overwritable ring buffer Wang Nan
2016-03-28  6:41 ` [PATCH 1/4] perf core: Introduce new ioctl options to pause and resume " Wang Nan
2016-03-28 10:15   ` [PATCH][manpages 1/2] perf_event_open.2: Document PERF_EVENT_IOC_PAUSE_OUTPUT Wang Nan
2016-03-28 10:15     ` Wang Nan
2016-10-21  8:56     ` Michael Kerrisk (man-pages)
2016-10-21  8:56       ` Michael Kerrisk (man-pages)
2016-10-21 14:37       ` Vince Weaver
2016-10-21 14:37         ` Vince Weaver
2016-10-21 14:49         ` Michael Kerrisk (man-pages)
2016-10-21 14:49           ` Michael Kerrisk (man-pages)
2016-03-29  0:27   ` [PATCH 1/4] perf core: Introduce new ioctl options to pause and resume ring buffer Alexei Starovoitov
2016-03-29  1:10     ` Wangnan (F)
2016-03-29  2:05     ` [PATCH 1/4 fix] " Wang Nan
2016-03-29  4:39       ` Alexei Starovoitov
2016-03-29 12:54   ` [PATCH 1/4] " Peter Zijlstra
2016-03-29 12:55     ` Peter Zijlstra
2016-03-30  1:57     ` Wangnan (F)
2016-03-30  6:46       ` Peter Zijlstra
2016-03-31  9:26   ` [tip:perf/core] perf/ring_buffer: Introduce new ioctl options to pause and resume the ring-buffer tip-bot for Wang Nan
2016-03-28  6:41 ` [PATCH 2/4] perf core: Set event's default overflow_handler Wang Nan
2016-03-31  9:26   ` [tip:perf/core] perf/core: Set event's default ::overflow_handler() tip-bot for Wang Nan
2016-03-28  6:41 ` [PATCH 3/4] perf core: Prepare writing into ring buffer from end Wang Nan
2016-03-29  0:25   ` Alexei Starovoitov
2016-03-31  9:26   ` [tip:perf/core] perf/ring_buffer: Prepare writing into the ring-buffer from the end tip-bot for Wang Nan
2016-03-28  6:41 ` [PATCH 4/4] perf core: Add backward attribute to perf event Wang Nan
2016-03-28 10:16   ` [PATCH][manpages 2/2] perf_event_open.2: Document write_backward Wang Nan
2016-03-28 10:16     ` Wang Nan
2016-10-21  8:57     ` Michael Kerrisk (man-pages)
2016-03-29  0:28   ` [PATCH 4/4] perf core: Add backward attribute to perf event Alexei Starovoitov
2016-03-29  2:01   ` Wangnan (F)
2016-03-29  4:59     ` Alexei Starovoitov
2016-03-29  5:59       ` Wangnan (F) [this message]
2016-03-29 14:04   ` Peter Zijlstra
2016-03-30  2:28     ` Wangnan (F)
2016-03-30  2:38       ` Wangnan (F)
2016-04-05 14:05         ` Wangnan (F)
2016-04-07  9:45     ` Wangnan (F)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56FA19DD.6040708@huawei.com \
    --to=wangnan0@huawei.com \
    --cc=acme@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=hekuang@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pi3orama@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.