Re: [PATCH] ftrace: don't increment @pos in g_start()

From: Wang Liming
Date: Sun Jun 21 2009 - 23:28:51 EST

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

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)

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

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at