* [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
@ 2011-10-20 7:43 Ricardo Ribalda Delgado
2011-10-20 7:54 ` Pekka Enberg
0 siblings, 1 reply; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2011-10-20 7:43 UTC (permalink / raw)
To: a.p.zijlstra, paulus, mingo, acme, anton, penberg, daahern,
linux-kernel
Cc: 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 7:43 [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size Ricardo Ribalda Delgado
@ 2011-10-20 7:54 ` Pekka Enberg
2011-10-20 8:14 ` Cyrill Gorcunov
0 siblings, 1 reply; 7+ messages in thread
From: Pekka Enberg @ 2011-10-20 7:54 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: a.p.zijlstra, paulus, mingo, acme, anton, daahern, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 7:54 ` Pekka Enberg
@ 2011-10-20 8:14 ` Cyrill Gorcunov
2011-10-20 8:45 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 7+ messages in thread
From: Cyrill Gorcunov @ 2011-10-20 8:14 UTC (permalink / raw)
To: Pekka Enberg
Cc: Ricardo Ribalda Delgado, a.p.zijlstra, paulus, mingo, acme, anton,
daahern, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 8:14 ` Cyrill Gorcunov
@ 2011-10-20 8:45 ` Ricardo Ribalda Delgado
2011-10-20 8:58 ` Cyrill Gorcunov
2011-10-20 9:14 ` Pekka Enberg
0 siblings, 2 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2011-10-20 8:45 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Pekka Enberg, a.p.zijlstra, paulus, mingo, acme, anton, daahern,
linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 8:45 ` Ricardo Ribalda Delgado
@ 2011-10-20 8:58 ` Cyrill Gorcunov
2011-10-20 9:14 ` Pekka Enberg
1 sibling, 0 replies; 7+ messages in thread
From: Cyrill Gorcunov @ 2011-10-20 8:58 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Pekka Enberg, a.p.zijlstra, paulus, mingo, acme, anton, daahern,
linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 8:45 ` Ricardo Ribalda Delgado
2011-10-20 8:58 ` Cyrill Gorcunov
@ 2011-10-20 9:14 ` Pekka Enberg
2011-10-21 15:23 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 7+ messages in thread
From: Pekka Enberg @ 2011-10-20 9:14 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Cyrill Gorcunov, a.p.zijlstra, paulus, mingo, acme, anton,
daahern, linux-kernel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size
2011-10-20 9:14 ` Pekka Enberg
@ 2011-10-21 15:23 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-21 15:23 UTC (permalink / raw)
To: Pekka Enberg
Cc: Ricardo Ribalda Delgado, Cyrill Gorcunov, a.p.zijlstra, paulus,
mingo, anton, daahern, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-10-21 15:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-20 7:43 [PATCH] tools/perf: Increase symbol KSYM_NAME_LEN size Ricardo Ribalda Delgado
2011-10-20 7:54 ` Pekka Enberg
2011-10-20 8:14 ` Cyrill Gorcunov
2011-10-20 8:45 ` Ricardo Ribalda Delgado
2011-10-20 8:58 ` Cyrill Gorcunov
2011-10-20 9:14 ` Pekka Enberg
2011-10-21 15:23 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox