Date: Tue, 24 Nov 2015 12:40:25 -0300
From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: David Ahern <dsahern@gmail.com>
Cc: Yunlong Song <yunlong.song@huawei.com>, a.p.zijlstra@chello.nl,
        paulus@samba.org, mingo@redhat.com, linux-kernel@vger.kernel.org,
        wangnan0@huawei.com, namhyung@kernel.org, ast@kernel.org,
        masami.hiramatsu.pt@hitachi.com, kan.liang@intel.com,
        adrian.hunter@intel.com, jolsa@kernel.org, bp@alien8.de,
        jean.pihet@linaro.org, rric@kernel.org, xiakaixu@huawei.com,
        hekuang@huawei.com
Subject: Re: [PATCH] perf record: Add snapshot mode support for perf's
 regular events
Message-ID: <20151124154025.GF18140@kernel.org>
References: <1448373632-8806-1-git-send-email-yunlong.song@huawei.com>
 <1448373632-8806-2-git-send-email-yunlong.song@huawei.com>
 <56547D01.8020606@gmail.com>
 <20151124152023.GE18140@kernel.org>
 <56548117.9080704@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <56548117.9080704@gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3406
Lines: 96

Em Tue, Nov 24, 2015 at 08:24:07AM -0700, David Ahern escreveu:
> On 11/24/15 8:20 AM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Nov 24, 2015 at 08:06:41AM -0700, David Ahern escreveu:
> >>On 11/24/15 7:00 AM, Yunlong Song wrote:
> >>>+static int record__write(struct record *rec, void *bf, size_t size)
> >>>+{
> >>>+	if (rec->memory.size && memory_enabled) {
> >>>+		if (perf_memory__write(&rec->memory, bf, size) < 0) {
> >>>+			pr_err("failed to write memory data, error: %m\n");
> >>>+			return -1;
> >>>+		}
> >>>+	} else {
> >>>+		if (perf_data_file__write(rec->session->file, bf, size) < 0) {
> >>>+			pr_err("failed to write perf data, error: %m\n");
> >>>+			return -1;
> >>>+		}
> >>>+		rec->bytes_written += size;
> >>>  	}
> >>>
> >>>-	rec->bytes_written += size;
> >>>  	return 0;
> >>>  }
> >>>
> >>>@@ -86,6 +214,8 @@ static int record__mmap_read(struct record *rec, int idx)
> >>>  	if (old == head)
> >>>  		return 0;
> >>>
> >>>+	memory_enabled = 1;
> >>>+
> >>>  	rec->samples++;
> >>>
> >>>  	size = head - old;
> >>>@@ -113,6 +243,7 @@ static int record__mmap_read(struct record *rec, int idx)
> >>>  	md->prev = old;
> >>>  	perf_evlist__mmap_consume(rec->evlist, idx);
> >>>  out:
> >>>+	memory_enabled = 0;
> >>>  	return rc;
> >>>  }
> >>>
> >>
> >>So you are basically ignoring all samples until SIGUSR2 is received. That
> >
> >No, he is not, its just that his code is difficult to follow, has to be
> >rewritten, but he is ignoring just PERF_RECORD_SAMPLE events, so it
> >will..

> >>means the resulting data file will have limited history of task events for

> >... have a complete history of task events, since PERF_RECORD_FORK, etc
> >are not being ignored.

> >No?
 
> perf-record does not process events, it only writes to a file. If that is
> skipped then it skips all events regardless of type.

perf-record without his patch? yes, but with his patch it does:

    __cmd_record()
        for (;;)
            record__mmap_read_all()
                record__write()
                    perf_memory__write()
			event = (union perf_event *)(memory->start + memory->head + skip);
                        if (event->header.type != PERF_RECORD_SAMPLE) {
                            if (buf_to_file(rec, memory->start, memory->size,
                        }

I almost thought that I had been fooled by the difficulty to follow his
patch and was forgetting that 'perf record' doesn't processes events,
and hasn't done so for a very good reason: to reduce its impact on the
observed workload, but that ain't so, no?

So, when not snapshotting, what you said remains true:

static int record__write(struct record *rec, void *bf, size_t size)
{
       if (rec->memory.size && memory_enabled) {
	        if (perf_memory__write(&rec->memory, bf, size) < 0) {
                       pr_err("failed to write memory data, error: %m\n");
	                return -1;
               }
       } else {
               if (perf_data_file__write(rec->session->file, bf, size) < 0) {

I'll continue taking the else branch and in that case, no events are
processed, we just dump that bf into the rec->session->file and go on
with life.

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