lkml.org 
[lkml]   [2009]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ftrace: don't increment @pos in g_start()
Li Zefan wrote:
> Wang Liming wrote:
>> Li Zefan wrote:
>>> Li Zefan wrote:
>>>> Liming Wang wrote:
>>>>> how about this one?
>>>>>
>>>> Yeah, this should work, and cleaner than my version.
>>>>
>>> Hmmm, the patch is cleaner in diffstat but the resulted code
>>> isn't..
>>>
>>> After yours:
>>> text data bss dec hex filename
>>> 14879 5480 4240 24599 6017 kernel/trace/ftrace.o
>>>
>>> After mine:
>>> text data bss dec hex filename
>>> 14873 5480 4240 24593 6011 kernel/trace/ftrace.o
>> Hmmm, if you prefer to smaller target size, I don't care.
>> But in my system, I got the same size:
>>
>> text data bss dec hex filename
>> 14330 5019 104 19453 4bfd kernel/trace/ftrace.o
>>
>> I use objdump to compute the actual size of all modified functions:
>>
>> After mine:
>> func size
>> g_start 0x50
>> g_next 0x70
>>
>> After yours:
>> func size
>> __g_next 0x70
>> g_next 0x20
>> g_start 0x30
>>
>> I used Steve git tree and commit e482f8395f215e0ad6557b2722cd9b9b308035c4.
>> My gcc version is :
>> gcc version 4.2.4
>>
>> I don't know where the difference.
>>
>
> Maybe because of different gcc versions:
>
> # gcc --version
> gcc (GCC) 4.1.2 20070925 (Red Hat 4.1.2-33)
Maybe.

>
> The point is, I don't see how the patch you posted is better than
> mine. And it's fine for me to pick up yours if it's indeed better.
OK, it's fine to me to pick up yours. Nothing different.
Back to your patch:

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index ec18bb4..2c1c761 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2492,32 +2492,30 @@ int ftrace_graph_count;
> unsigned long ftrace_graph_funcs[FTRACE_GRAPH_MAX_FUNCS] __read_mostly;
>
> static void *
> -g_next(struct seq_file *m, void *v, loff_t *pos)
> +__g_next(struct seq_file *m, loff_t *pos)
> {
> unsigned long *array = m->private;
> - int index = *pos;
>
> - (*pos)++;
> + /* Nothing, tell g_show to print all functions are enabled */
> + if (!ftrace_graph_count && !*pos)
> + return (void *)1;
I think this checking should be put back to g_start, because it's only necessary
for g_start.

Liming Wang

>



\
 
 \ /
  Last update: 2009-06-22 05:31    [W:0.050 / U:0.944 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site