Messages in this thread | | | Date | Tue, 23 Jun 2009 15:19:46 +0800 | From | Wang Liming <> | Subject | Re: [PATCH 5/5] tracing: reset iterator in t_start() |
| |
Li Zefan wrote: >> Another version: >> Since we have saved current (struct tracer *) in m->private in .next, in >> .start, we don't need to call .next to find the one that should be >> printed in 2nd or nth time. >> > > I don't like this for 2 reasons. > > 1. It's strange that @pos is not incremented in next(). Yes, it's strang, but we know that @pos sometimes is not necessary, such in this position.
> > 2. > t_stop() > mutex_unlock() > unregister_tracer(t) > t_start() > mutex_lock() > t = m->private > ... > t = t-next. > > We access t->next though @t was unregistered. This is not > good, though it does no harm here. OK, it's a realy race problem if we call unregister_tracer. btw: who realy calls this function? :)
Liming Wang > >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c >> index cae34c6..02cdccc 100644 >> --- a/kernel/trace/trace.c >> +++ b/kernel/trace/trace.c >> @@ -2055,8 +2055,6 @@ t_next(struct seq_file *m, void *v, loff_t *pos) >> { >> struct tracer *t = m->private; >> >> - (*pos)++; >> - >> if (t) >> t = t->next; >> >> @@ -2068,11 +2066,8 @@ t_next(struct seq_file *m, void *v, loff_t *pos) >> static void *t_start(struct seq_file *m, loff_t *pos) >> { >> struct tracer *t = m->private; >> - loff_t l = 0; >> >> mutex_lock(&trace_types_lock); >> - for (; t && l < *pos; t = t_next(m, t, &l)) >> - ; >> >> return t; >> } >> >> >
| |