Messages in this thread |  | | Date | Sat, 30 Nov 2013 08:58:56 -0700 | From | David Ahern <> | Subject | Re: [PATCH 2/8] perf symbols: Move idle syms check from top to generic function |
| |
On 11/28/13, 1:36 AM, Namhyung Kim wrote: > Hi David, > > Just minor nits below.. > > On Mon, 18 Nov 2013 13:32:45 -0700, David Ahern wrote: >> Allows list of idle symbols to be leveraged by other commands, >> such as the upcoming timehist command. > > [SNIP] >> +bool symbol__is_idle(struct symbol *sym) >> +{ >> + const char * const idle_symbols[] = { > > Wouldn't it better making it static? It seems to build a table > everytime otherwise.
Checked a couple of gcc's (4.3.2 32-bit and 4.6.3 64-bit) and both put idle_symbols in read-only data section. e.g.,
0000000000000020 r idle_symbols.13404
So I think it is fine as is.
> > >> + "cpu_idle", >> + "intel_idle", >> + "default_idle", >> + "native_safe_halt", >> + "enter_idle", >> + "exit_idle", >> + "mwait_idle", >> + "mwait_idle_with_hints", >> + "poll_idle", >> + "ppc64_runlatch_off", >> + "pseries_dedicated_idle_sleep", >> + NULL >> + }; >> + >> + int i; >> + >> + if (!sym) >> + return false; >> + >> + for (i = 0; idle_symbols[i]; i++) { > > Also we can use ARRAY_SIZE() here and let the last NULL go IMHO.
I copied what was in builtin-top.c.
David
|  |