[PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Ricardo Ribalda Delgado]

Fglrx propietary driver has symbol names over 128 chars (:S). This
breaks the function kallsyms__parse.

This fix increases the size of KSYM_NAME_LEN, so kallsyms__parse can
work on such kernels.

The only counterparty, is that such function requires 128 more bytes to
work.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 tools/perf/util/symbol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 40eeaf0..d4f8750 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -24,7 +24,7 @@
 #include <sys/utsname.h>
 
 #ifndef KSYM_NAME_LEN
-#define KSYM_NAME_LEN 128
+#define KSYM_NAME_LEN 256
 #endif
 
 #ifndef NT_GNU_BUILD_ID
-- 
1.7.7

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Pekka Enberg]

On Thu, Oct 20, 2011 at 10:43 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Fglrx propietary driver has symbol names over 128 chars (:S). This
> breaks the function kallsyms__parse.
>
> This fix increases the size of KSYM_NAME_LEN, so kallsyms__parse can
> work on such kernels.
>
> The only counterparty, is that such function requires 128 more bytes to
> work.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  tools/perf/util/symbol.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 40eeaf0..d4f8750 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -24,7 +24,7 @@
>  #include <sys/utsname.h>
>
>  #ifndef KSYM_NAME_LEN
> -#define KSYM_NAME_LEN 128
> +#define KSYM_NAME_LEN 256
>  #endif
>
>  #ifndef NT_GNU_BUILD_ID
> --
> 1.7.7

Is there some specified maximum length for symbols (e.g. in ELF spec)?
I'm OK with the patch but I'd prefer we didn't bump up the number
blindly there's something "official" number we can use.

                        Pekka

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Cyrill Gorcunov]

On Thu, Oct 20, 2011 at 10:54:40AM +0300, Pekka Enberg wrote:
...
> 
> Is there some specified maximum length for symbols (e.g. in ELF spec)?
> I'm OK with the patch but I'd prefer we didn't bump up the number
> blindly there's something "official" number we can use.
> 
>                         Pekka

Elf restricts offsets in string table by 4 bytes, so probably the option
is to have complete .strtab being read somewhere and use indices when needed.
(note I didn't check where perf tool need those names).

	Cyrill

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Ricardo Ribalda Delgado]

That is 4gb size :S..... I have tried making a dummy program

int tttttt......ttttt (int a, int b){
return a+b;
}

int main(int arg, char *[]argv){
    return tttt....tttt(1,2);
}

and it has worked at least until t^16384 both in gcc and in gdb, and I
guess it would have continue working until much more.

Right now, I think we can increase it until 512, to be on the safe
side, and keep it simple. What do you think?

  best regards

On Thu, Oct 20, 2011 at 10:14, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On Thu, Oct 20, 2011 at 10:54:40AM +0300, Pekka Enberg wrote:
> ...
>>
>> Is there some specified maximum length for symbols (e.g. in ELF spec)?
>> I'm OK with the patch but I'd prefer we didn't bump up the number
>> blindly there's something "official" number we can use.
>>
>>                         Pekka
>
> Elf restricts offsets in string table by 4 bytes, so probably the option
> is to have complete .strtab being read somewhere and use indices when needed.
> (note I didn't check where perf tool need those names).
>
>        Cyrill
>



-- 
Ricardo Ribalda

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Cyrill Gorcunov]

On Thu, Oct 20, 2011 at 10:45:01AM +0200, Ricardo Ribalda Delgado wrote:
> That is 4gb size :S..... I have tried making a dummy program
> 
> int tttttt......ttttt (int a, int b){
> return a+b;
> }
> 
> int main(int arg, char *[]argv){
>     return tttt....tttt(1,2);
> }
> 
> and it has worked at least until t^16384 both in gcc and in gdb, and I
> guess it would have continue working until much more.
> 
> Right now, I think we can increase it until 512, to be on the safe
> side, and keep it simple. What do you think?
> 

I've no idea how perf uses them, so I think I have no rights to
advise here ;) If this has no performance impact then sure, looks
good.

	Cyrill

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Pekka Enberg]

On Thu, Oct 20, 2011 at 11:45 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> That is 4gb size :S..... I have tried making a dummy program
>
> int tttttt......ttttt (int a, int b){
> return a+b;
> }
>
> int main(int arg, char *[]argv){
>    return tttt....tttt(1,2);
> }
>
> and it has worked at least until t^16384 both in gcc and in gdb, and I
> guess it would have continue working until much more.
>
> Right now, I think we can increase it until 512, to be on the safe
> side, and keep it simple. What do you think?

If the limit is really 4G, I'm fine with whatever is the smallest
practical size. If 256 bytes works for you:

Acked-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size

[Arnaldo Carvalho de Melo]

Em Thu, Oct 20, 2011 at 12:14:06PM +0300, Pekka Enberg escreveu:
> On Thu, Oct 20, 2011 at 11:45 AM, Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:
> > and it has worked at least until t^16384 both in gcc and in gdb, and I
> > guess it would have continue working until much more.
> >
> > Right now, I think we can increase it until 512, to be on the safe
> > side, and keep it simple. What do you think?
> 
> If the limit is really 4G, I'm fine with whatever is the smallest
> practical size. If 256 bytes works for you:
> 
> Acked-by: Pekka Enberg <penberg@kernel.org>

Thanks guys, will apply the first patch, bumping it to 256.

- Arnaldo