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

[Wang Liming <liming.wang@windriver.com>]

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.

Liming Wang

> 
> 
>>> It's wrong to increment @pos in g_start(). It causes some entries
>>> lost when reading set_graph_function, if the output of the file
>>> is large than PAGE_SIZE.
>>>
>>> [ Impact: fix missing entries when reading set_graph_function ]
>>>
>>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>>> Signed-off-by: Liming Wang <liming.wang@windriver.com>
>>> ---
>>>  kernel/trace/ftrace.c |   14 +++++---------
>>>  1 files changed, 5 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>>> index 134e580..1beaac6 100644
>>> --- a/kernel/trace/ftrace.c
>>> +++ b/kernel/trace/ftrace.c
>>> @@ -2495,29 +2495,25 @@ static void *
>>>  g_next(struct seq_file *m, void *v, loff_t *pos)
>>>  {
>>>  	unsigned long *array = m->private;
>>> -	int index = *pos;
>>>  
>>> -	(*pos)++;
>>> +	if (v)
>>> +		(*pos)++;
>>>  
>>> -	if (index >= ftrace_graph_count)
>>> +	if (*pos >= ftrace_graph_count)
>>>  		return NULL;
>>>  
>>> -	return &array[index];
>>> +	return &array[*pos];
>>>  }
>>>  
>>>  static void *g_start(struct seq_file *m, loff_t *pos)
>>>  {
>>> -	void *p = NULL;
>>> -
>>>  	mutex_lock(&graph_lock);
>>>  
>>>  	/* Nothing, tell g_show to print all functions are enabled */
>>>  	if (!ftrace_graph_count && !*pos)
>>>  		return (void *)1;
>>>  
>>> -	p = g_next(m, p, pos);
>>> -
>>> -	return p;
>>> +	return g_next(m, NULL, pos);
>>>  }
>>>  
>>>  static void g_stop(struct seq_file *m, void *p)
>