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