Re: [PATCH 1/6] perf timechart: always try to print at least 15 tasks

From: Namhyung Kim
Date: Fri Nov 01 2013 - 02:06:31 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/