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

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

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