All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
Cc: "Lukáš Doktor" <ldoktor@redhat.com>,
	ehabkost@redhat.com, "QEMU Developers" <qemu-devel@nongnu.org>,
	"Aleksandar Markovic" <aleksandar.qemu.devel@gmail.com>,
	crosa@redhat.com, "Richard Henderson" <rth@twiddle.net>
Subject: Re: [PATCH 3/3] scripts/performance: Add perf_top_25.py script
Date: Wed, 17 Jun 2020 18:35:04 +0100	[thread overview]
Message-ID: <877dw5wq7r.fsf@linaro.org> (raw)
In-Reply-To: <CALTWKrVovkoQvNFxYac2eOV7Cf+K_RA+1-Gn=3AnL8dJLemTyQ@mail.gmail.com>


Ahmed Karaman <ahmedkhaledkaraman@gmail.com> writes:

> On Wed, Jun 17, 2020 at 2:21 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> > +
>> > +# Run perf repcord and report
>> > +os.system('sudo perf record {} {} 2> /dev/null \
>> > +            && sudo perf report --stdio > tmp.perf.data'
>> > +          .format(qemu_path, executable))
>>
>> Why sudo?
>
> This is the default requirement by perf. You can modify the
> kernel.perf_event_paranoid setting to run without root privileges.

Right - which I do as a developer. It would be rude to sudo things if
you don't need to because then you end up running your potentially
un-trusted application with root privileges.

Could we either probe for the requirement or require an explicit sudo
flag which we can prompt for if it fails?

>
>> Also redirecting just stderr? why?
>
> Perf, as well as Valgrind, print their output on stderr not stdout.

Right so I think a bit of splitting apart and use of subprocess can make
this cleaner and not involve quite so much being done with shell
redirection in one invocation.

>
>> I think you could separate the steps (as well as use the subprocess
>> api).
>
> Noted!
>
>> Again os.unlink()
>
> Noted!


-- 
Alex Bennée


  reply	other threads:[~2020-06-17 17:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16 23:12 [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions Ahmed Karaman
2020-06-16 23:12 ` [PATCH 1/3] MAINTAINERS: Add 'Miscellaneous' section Ahmed Karaman
2020-06-17  5:35   ` Aleksandar Markovic
2020-06-17  5:51   ` Aleksandar Markovic
2020-06-17 12:01   ` Alex Bennée
2020-06-16 23:12 ` [PATCH 2/3] scripts/performance: Add callgrind_top_25.py script Ahmed Karaman
2020-06-17  5:42   ` Aleksandar Markovic
2020-06-17 12:16   ` Alex Bennée
2020-06-17 16:08     ` Ahmed Karaman
2020-06-16 23:12 ` [PATCH 3/3] scripts/performance: Add perf_top_25.py script Ahmed Karaman
2020-06-17  5:56   ` Aleksandar Markovic
2020-06-17 12:21   ` Alex Bennée
2020-06-17 16:15     ` Ahmed Karaman
2020-06-17 17:35       ` Alex Bennée [this message]
2020-06-17 18:21         ` Ahmed Karaman
2020-06-18 15:07           ` Aleksandar Markovic
2020-06-16 23:35 ` [PATCH 0/3] Add Scripts for Finding Top 25 Executed Functions no-reply
2020-06-17 13:53 ` Eric Blake
2020-06-17 15:34   ` Alex Bennée
2020-06-17 16:16     ` Aleksandar Markovic
2020-06-17 17:42       ` Alex Bennée
2020-06-17 16:16   ` Ahmed Karaman

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=877dw5wq7r.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=ahmedkhaledkaraman@gmail.com \
    --cc=aleksandar.qemu.devel@gmail.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=ldoktor@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /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.