Re: [PATCH 2/5] tracepoint: Convert process iteration to use for_each_process_thread()

[Steven Rostedt]

On Wed, 9 Apr 2014 16:28:35 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:

> ----- Original Message -----
> > From: "Frederic Weisbecker" <fweisbec@xxxxxxxxx>
> > To: "LKML" <linux-kernel@xxxxxxxxxxxxxxx>
> > Cc: "Frederic Weisbecker" <fweisbec@xxxxxxxxx>, "Andrew Morton" <akpm@xxxxxxxxxxxxxxxxxxxx>, "Ingo Molnar"
> > <mingo@xxxxxxxxxx>, "Mathieu Desnoyers" <mathieu.desnoyers@xxxxxxxxxxxx>, "Oleg Nesterov" <oleg@xxxxxxxxxx>, "Steven
> > Rostedt" <rostedt@xxxxxxxxxxx>
> > Sent: Wednesday, April 9, 2014 12:11:19 PM
> > Subject: [PATCH 2/5] tracepoint: Convert process iteration to use for_each_process_thread()
> > 
> > do_each_thread/while_each_thread iterators are deprecated by
> > for_each_thread/for_each_process_thread() APIs.
> > 
> > Lets convert the callers in the tracepoint code. The ultimate
> > goal is to remove the struct task_struct::thread_group field and
> > the corresponding do_each_thread/while_each_thread iterators that are
> > RCU unsafe.
> > 
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> > Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > Signed-off-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> > ---
> >  kernel/tracepoint.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index fb0a38a..00a7e8b 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -561,15 +561,15 @@ static int sys_tracepoint_refcount;
> >  void syscall_regfunc(void)
> >  {
> >  	unsigned long flags;
> > -	struct task_struct *g, *t;
> > +	struct task_struct *p, *t;
> >  
> >  	if (!sys_tracepoint_refcount) {
> >  		read_lock_irqsave(&tasklist_lock, flags);
> > -		do_each_thread(g, t) {
> > +		for_each_process_thread(p, t) {
> 
> What are the locking rules for for_each_process_thread() ?
> 
> Is it required to hold RCU read-side lock ? (it's not the case here)
> 
> Is tasklist_lock read-side lock sufficient ?

The tasklist_lock is all that is needed. Nothing about the list will
change when that's held. Ideally we would like to start converting
things to rcu, where it's OK if the list actually changes while you are
iterating.

But if you require the list to stay the same while you read all the
tasks, then you need the bigger hammer (tasklist_lock).

Also, this is just about iterating the tasklist list. If something
within the loop requires rcu, then that's a different story.

-- Steve

> 
> A quick glance at those for_each iterator defines in sched.h was not
> helpful in finding this information.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/