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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 01 Nov 2013 15:06:09 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Stanislav Fomichev <stfomichev@...dex-team.ru>
Cc:	a.p.zijlstra@...llo.nl, paulus@...ba.org, mingo@...hat.com,
	acme@...stprotocols.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] perf timechart: always try to print at least 15 tasks

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/