All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Stanislav Fomichev <stfomichev@yandex-team.ru>
Cc: a.p.zijlstra@chello.nl, paulus@samba.org, mingo@redhat.com,
	acme@ghostprotocols.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] perf timechart: always try to print at least 15 tasks
Date: Fri, 01 Nov 2013 15:06:09 +0900	[thread overview]
Message-ID: <878ux864ha.fsf@sejong.aot.lge.com> (raw)
In-Reply-To: <20131029092433.GA3406@stfomichev-desktop> (Stanislav Fomichev's message of "Tue, 29 Oct 2013 13:24:33 +0400")

Hi Stanislav,

On Tue, 29 Oct 2013 13:24:33 +0400, Stanislav Fomichev wrote:
>> 1. introduce while loop: it's a behavioral change so that it can be a
>> separate patch.  But it seems not checking the process filter - in that
>> case the loop is almost useless IMHO.
> Could you please elaborate on 'not checking the process filter'?

I meant it doesn't check whether process filter is set.  If the filter
is set calling determine_display_tasks() multiple times is meaningless
since it doesn't check the threashold at all.

>
> This loop is for the case when process filter is not set, but when the
> filter is set it should also work (because determine_display_tasks calls
> determine_display_tasks_filtered in case of process filter).
> We just check the return value and loop while number of tasks is not
> within the desired range (or thresh is zero).
>
>> 2. new -n option: it should update Documentation/perf-timechart.txt
>> also.  And the long option name "number" is too general.
> Does renaming "number" to "proc-num" sounds ok?

Much better.

>
>> 3. two if(proc_num): what is this?  Is it for patch 2?
> That's for the '-n 0' case, to completely skip tasks information.
>
> Thanks for your comments, I'll split this patch into two parts and
> update the docs.

Thanks,
Namhyung

  reply	other threads:[~2013-11-01  6:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-22 10:56 [PATCH 0/6] perf timechart improvements Stanislav Fomichev
2013-10-22 10:56 ` [PATCH 1/6] perf timechart: always try to print at least 15 tasks Stanislav Fomichev
2013-10-29  7:50   ` Namhyung Kim
2013-10-29  9:24     ` Stanislav Fomichev
2013-11-01  6:06       ` Namhyung Kim [this message]
2013-10-22 10:56 ` [PATCH 2/6] perf timechart: use proc_num to implement --power-only Stanislav Fomichev
2013-10-29  8:03   ` Namhyung Kim
2013-10-29  9:28     ` Stanislav Fomichev
2013-11-01  6:07       ` Namhyung Kim
2013-10-22 10:56 ` [PATCH 3/6] perf timechart: add support for displaying only tasks related data Stanislav Fomichev
2013-10-29  8:10   ` Namhyung Kim
2013-10-29  9:33     ` Stanislav Fomichev
2013-11-01  6:10       ` Namhyung Kim
2013-10-22 10:56 ` [PATCH 4/6] perf timechart: group figures and add title with details Stanislav Fomichev
2013-10-29  8:15   ` Namhyung Kim
2013-10-29  9:35     ` Stanislav Fomichev
2013-10-22 10:56 ` [PATCH 5/6] perf timechart: add support for -P and -T in timechart recording Stanislav Fomichev
2013-10-29  8:27   ` Namhyung Kim
2013-10-29  9:51     ` Stanislav Fomichev
2013-10-22 10:56 ` [PATCH 6/6] perf timechart: add backtrace support Stanislav Fomichev
2013-10-29  8:41   ` Namhyung Kim

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=878ux864ha.fsf@sejong.aot.lge.com \
    --to=namhyung@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=stfomichev@yandex-team.ru \
    /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.