linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: jolsa@redhat.com
Cc: acme@kernel.org, dzickus@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: perf overlapping maps...
Date: Wed, 24 Oct 2018 14:30:45 -0700 (PDT)	[thread overview]
Message-ID: <20181024.143045.1124161066942465584.davem@davemloft.net> (raw)
In-Reply-To: <20181024113416.GA26027@krava>

From: Jiri Olsa <jolsa@redhat.com>
Date: Wed, 24 Oct 2018 13:34:16 +0200

> On Tue, Oct 23, 2018 at 11:15:03AM -0700, David Miller wrote:
>> From: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Date: Tue, 23 Oct 2018 15:05:03 -0300
>> 
>> > IIRC this was first done for 'perf record', where we have to stash those
>> > events in the perf.data file, to then, later, 'perf report' to process
>> > those, so when working on 'perf top', it just reuses that machinery.
>> > 
>> > Sure, with some love and care 'perf top' could do better and update all
>> > the data structures directly :-)
>> 
>> Thanks for the history, it is useful information :)
>> 
>> > Anyway, have you guys considered tweaking using event->header.misc |=
>> > PERF_RECORD_MISC_USER? The kernel leaves that as zero for the
>> > PERF_RECORD_FORK it emits:
>> 
>> I really would like to steer the approach away from using UAPI
>> perf_event fields in an internal way.
>> 
>> I am really very sorry for suggesting such a scheme myself in the
>> first place.  It really was a bad idea upon much consideration.
>> 
>> The synthetic fork is not really a fork, it's more like a "create".
>> 
>> And this fundamental semantic difference is why we have all of these
>> issues wrt. handling COMM and parent map inheritance.
>> 
>> There is also a bunch of non-trivial code to deal with whether we
>> synthetically create the child or the parent first, wrt. finding
>> thread leaders and parent threads.
>> 
>> What I'm trying to say is that there is a clean design based solution
>> hiding somewhere in here and I'd like to find it :-)
> 
> how about adding a data file marker/event when the synthesized
> portion of data is over
> 
> attached patch adds an 'SYNTHESIZE_END' event and prevents
> parent's maps cloning on fork until that event is found
> 
> we would need more code to stay backward compatible, which
> I did not include.. just to cleanly outline the solution

I appreciate everyone reiterating the ABI issue.  Wherein, perf record
stashes these events into a file and later analysis tools need to
interpret them.

Considering this specifically, it is very tempting to make a new event
representing this thread "CREATE" operation.

But, if we make a new event then existing tools will break because
they will see this unrecognizable event and not create the threads.

From this perspective setting a misc flag in the FORK event might in
fact be the best compromise.

Existing tools will still create the thread, in exactly the same
existing way.  This keeps things working as well as they do currently.

So my vote returns to emitting FORK with the special COMM flag set in
the misc flags.  And we document this properly of course.

Any objections?  I'll cook up a formal patch meanwhile in case we do
agree.

      reply	other threads:[~2018-10-24 21:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-20  4:05 perf overlapping maps David Miller
2018-10-20  4:44 ` David Miller
2018-10-22 14:07   ` Don Zickus
2018-10-22 16:16     ` Jiri Olsa
2018-10-22 17:10       ` Don Zickus
2018-10-22 17:58       ` David Miller
2018-10-23  6:34         ` Jiri Olsa
2018-10-23 17:54           ` David Miller
2018-10-23 18:05             ` Arnaldo Carvalho de Melo
2018-10-23 18:15               ` David Miller
2018-10-23 19:27                 ` Arnaldo Carvalho de Melo
2018-10-24 11:34                 ` Jiri Olsa
2018-10-24 21:30                   ` David Miller [this message]

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=20181024.143045.1124161066942465584.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=acme@kernel.org \
    --cc=dzickus@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).