linux-kernel - Re: [PATCH] perf record: Add snapshot mode support for perf's regular events

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]

Date:	Tue, 24 Nov 2015 12:40:25 -0300
From:	Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:	David Ahern <dsahern@...il.com>
Cc:	Yunlong Song <yunlong.song@...wei.com>, a.p.zijlstra@...llo.nl,
	paulus@...ba.org, mingo@...hat.com, linux-kernel@...r.kernel.org,
	wangnan0@...wei.com, namhyung@...nel.org, ast@...nel.org,
	masami.hiramatsu.pt@...achi.com, kan.liang@...el.com,
	adrian.hunter@...el.com, jolsa@...nel.org, bp@...en8.de,
	jean.pihet@...aro.org, rric@...nel.org, xiakaixu@...wei.com,
	hekuang@...wei.com
Subject: Re: [PATCH] perf record: Add snapshot mode support for perf's
 regular events

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/