* ftrace does not work on cpus > 999.
@ 2010-10-16 13:15 Robin Holt
2010-10-16 14:45 ` Frederic Weisbecker
0 siblings, 1 reply; 6+ messages in thread
From: Robin Holt @ 2010-10-16 13:15 UTC (permalink / raw)
To: Steven Rostedt, Frederic Weisbecker, Ingo Molnar; +Cc: linux-kernel
Just started looking into using ftrace to identify a difficult to diagnose
userland set of processes when I noticed that the .../trace/per_cpu
directory only contains entries for cpus 0-999. I have not looked at
what else may not work. For the cpus that are listed, I am able to grep
trace buffer information so it is mostly working.
I will be away from email for most of today, but may get back to this
tonight. If there are any suggestions, I will happily give them a try.
Thanks,
Robin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ftrace does not work on cpus > 999.
2010-10-16 13:15 ftrace does not work on cpus > 999 Robin Holt
@ 2010-10-16 14:45 ` Frederic Weisbecker
2010-10-16 15:17 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Frederic Weisbecker @ 2010-10-16 14:45 UTC (permalink / raw)
To: Robin Holt; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel
On Sat, Oct 16, 2010 at 08:15:05AM -0500, Robin Holt wrote:
> Just started looking into using ftrace to identify a difficult to diagnose
> userland set of processes when I noticed that the .../trace/per_cpu
> directory only contains entries for cpus 0-999. I have not looked at
> what else may not work. For the cpus that are listed, I am able to grep
> trace buffer information so it is mostly working.
>
> I will be away from email for most of today, but may get back to this
> tonight. If there are any suggestions, I will happily give them a try.
>
> Thanks,
> Robin
Oh!
Can you please try that?
Thanks.
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 9ec59f5..3565f9e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3999,7 +3999,7 @@ static void tracing_init_debugfs_percpu(long cpu)
/* strlen(cpu) + MAX(log10(cpu)) + '\0' */
char cpu_dir[7];
- if (cpu > 999 || cpu < 0)
+ if (cpu < 0)
return;
sprintf(cpu_dir, "cpu%ld", cpu);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: ftrace does not work on cpus > 999.
2010-10-16 14:45 ` Frederic Weisbecker
@ 2010-10-16 15:17 ` Steven Rostedt
2010-10-16 16:16 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2010-10-16 15:17 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Robin Holt, Ingo Molnar, linux-kernel
On Sat, 2010-10-16 at 16:45 +0200, Frederic Weisbecker wrote:
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 9ec59f5..3565f9e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3999,7 +3999,7 @@ static void tracing_init_debugfs_percpu(long cpu)
> /* strlen(cpu) + MAX(log10(cpu)) + '\0' */
> char cpu_dir[7];
>
> - if (cpu > 999 || cpu < 0)
> + if (cpu < 0)
> return;
>
> sprintf(cpu_dir, "cpu%ld", cpu);
You need to change the size of cpu_dir, otherwise this will overflow.
The other case is to dynamically allocate cpu_dir.
char *cpu_dir;
int count = cpu;
int size;
if (cpu < 0)
return;
/* log10 sizeof cpu */
for (size = 0; count > 0; count /= 10, size++)
;
cpu_dir = kmalloc(size + 4, GFP_KERNEL);
if (!cpu_dir)
return;
snprintf(cpu_dir, size + 4, "cpu%ld", cpu);
[...]
kfree(cpu_dir);
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ftrace does not work on cpus > 999.
2010-10-16 15:17 ` Steven Rostedt
@ 2010-10-16 16:16 ` Ingo Molnar
2010-10-16 16:32 ` Steven Rostedt
2010-10-16 16:32 ` Geert Uytterhoeven
0 siblings, 2 replies; 6+ messages in thread
From: Ingo Molnar @ 2010-10-16 16:16 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Frederic Weisbecker, Robin Holt, Ingo Molnar, linux-kernel
* Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 2010-10-16 at 16:45 +0200, Frederic Weisbecker wrote:
>
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 9ec59f5..3565f9e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3999,7 +3999,7 @@ static void tracing_init_debugfs_percpu(long cpu)
> > /* strlen(cpu) + MAX(log10(cpu)) + '\0' */
> > char cpu_dir[7];
> >
> > - if (cpu > 999 || cpu < 0)
> > + if (cpu < 0)
> > return;
> >
> > sprintf(cpu_dir, "cpu%ld", cpu);
>
> You need to change the size of cpu_dir, otherwise this will overflow.
If you change it to 30 that ought to be enough, as long as cpu_id's fit
into u64. (I think we wont overflow that in my lifetime.)
> The other case is to dynamically allocate cpu_dir.
Please tell me that is a joke ...
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ftrace does not work on cpus > 999.
2010-10-16 16:16 ` Ingo Molnar
@ 2010-10-16 16:32 ` Steven Rostedt
2010-10-16 16:32 ` Geert Uytterhoeven
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2010-10-16 16:32 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Frederic Weisbecker, Robin Holt, Ingo Molnar, linux-kernel
On Sat, 2010-10-16 at 18:16 +0200, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Sat, 2010-10-16 at 16:45 +0200, Frederic Weisbecker wrote:
> >
> > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > index 9ec59f5..3565f9e 100644
> > > --- a/kernel/trace/trace.c
> > > +++ b/kernel/trace/trace.c
> > > @@ -3999,7 +3999,7 @@ static void tracing_init_debugfs_percpu(long cpu)
> > > /* strlen(cpu) + MAX(log10(cpu)) + '\0' */
> > > char cpu_dir[7];
> > >
> > > - if (cpu > 999 || cpu < 0)
> > > + if (cpu < 0)
> > > return;
> > >
> > > sprintf(cpu_dir, "cpu%ld", cpu);
> >
> > You need to change the size of cpu_dir, otherwise this will overflow.
>
> If you change it to 30 that ought to be enough, as long as cpu_id's fit
> into u64. (I think we wont overflow that in my lifetime.)
And might as well make it static then, as we seem to be doing with
sched_param.
>
> > The other case is to dynamically allocate cpu_dir.
>
> Please tell me that is a joke ...
Why? It handles all cases and is only executed once at boot up. I'll
admit it was a little over engineered, and setting a new max of 30 chars
should be fine too. But no reason for it to be a joke.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ftrace does not work on cpus > 999.
2010-10-16 16:16 ` Ingo Molnar
2010-10-16 16:32 ` Steven Rostedt
@ 2010-10-16 16:32 ` Geert Uytterhoeven
1 sibling, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2010-10-16 16:32 UTC (permalink / raw)
To: Ingo Molnar
Cc: Steven Rostedt, Frederic Weisbecker, Robin Holt, Ingo Molnar,
linux-kernel
On Sat, Oct 16, 2010 at 18:16, Ingo Molnar <mingo@elte.hu> wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> On Sat, 2010-10-16 at 16:45 +0200, Frederic Weisbecker wrote:
>>
>> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> > index 9ec59f5..3565f9e 100644
>> > --- a/kernel/trace/trace.c
>> > +++ b/kernel/trace/trace.c
>> > @@ -3999,7 +3999,7 @@ static void tracing_init_debugfs_percpu(long cpu)
>> > /* strlen(cpu) + MAX(log10(cpu)) + '\0' */
>> > char cpu_dir[7];
>> >
>> > - if (cpu > 999 || cpu < 0)
>> > + if (cpu < 0)
>> > return;
>> >
>> > sprintf(cpu_dir, "cpu%ld", cpu);
>>
>> You need to change the size of cpu_dir, otherwise this will overflow.
>
> If you change it to 30 that ought to be enough, as long as cpu_id's fit
Yep.
> into u64. (I think we wont overflow that in my lifetime.)
>> The other case is to dynamically allocate cpu_dir.
>
> Please tell me that is a joke ...
And if it isn't, use kasprintf() ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-16 16:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-16 13:15 ftrace does not work on cpus > 999 Robin Holt
2010-10-16 14:45 ` Frederic Weisbecker
2010-10-16 15:17 ` Steven Rostedt
2010-10-16 16:16 ` Ingo Molnar
2010-10-16 16:32 ` Steven Rostedt
2010-10-16 16:32 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox