lkml.org 
[lkml]   [2012]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: syscall_regfunc() && TIF_SYSCALL_TRACEPOINT
From
Date
On Fri, 2012-03-30 at 20:31 +0200, Oleg Nesterov wrote:
> Hello.
>
> I've looked at syscall_regfunc/unregfunc by accident, and I am
> a bit confused...
>
> void syscall_regfunc(void)
> {
> unsigned long flags;
> struct task_struct *g, *t;
>
> if (!sys_tracepoint_refcount) {
> read_lock_irqsave(&tasklist_lock, flags);
>
> Why _irqsave? write_lock(tasklist) needs to disable irqs, but read_
> doesn't. Any subtle reason I missed?

As long as an interrupt doesn't take the tasklist lock as write, we
don't. If it doesn't then we should be safe not to disable interrupts
here.

>
> do_each_thread(g, t) {
> /* Skip kernel threads. */
> if (t->mm)
>
> We should check PF_KTHREAD, not ->mm.

A lot of places test ->mm for kernel threads. Just search for ->mm in
kernel/sched/core.c


>
> set_tsk_thread_flag(t, TIF_SYSCALL_TRACEPOINT);
>
> But the main question is, can't we race with clone() and miss the
> new child? The new task is not "visible" to do_each_thread() until
> copy_process()->list_add_tail_rcu(thread_group/init_task.tasks).
>
> Don't we need something like the patch below?
>
> Oleg.
>
>
> --- x/kernel/fork.c
> +++ x/kernel/fork.c
> @@ -1446,7 +1446,12 @@ static struct task_struct *copy_process(
>
> total_forks++;
> spin_unlock(&current->sighand->siglock);
> +#ifdef CONFIG_TRACEPOINTS
> + if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> + set_tsk_thread_flag(p, TIF_SYSCALL_TRACEPOINT);

I'm not so worried about the set (although that should be done) but it
is entirely possible that we need a clear too. Leaving this flag set
would cause a task to take the overhead of tracing syscalls without ever
tracing them.

-- Steve


> +#endif
> write_unlock_irq(&tasklist_lock);
> +
> proc_fork_connector(p);
> cgroup_post_fork(p);
> if (clone_flags & CLONE_THREAD)




\
 
 \ /
  Last update: 2012-03-30 21:05    [W:0.113 / U:0.856 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site