Re: [PATCH 08/12] add trace events for each syscall entry/exit

From: Frederic Weisbecker <fweisbec@gmail.com>
To: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>,
	Jason Baron <jbaron@redhat.com>,
	linux-kernel@vger.kernel.org, mingo@elte.hu,
	laijs@cn.fujitsu.com, rostedt@goodmis.org, peterz@infradead.org,
	mathieu.desnoyers@polymtl.ca, jiayingz@google.com,
	mbligh@google.com, lizf@cn.fujitsu.com,
	Martin Schwidefsky <schwidefsky@de.ibm.com>
Subject: Re: [PATCH 08/12] add trace events for each syscall entry/exit
Date: Wed, 26 Aug 2009 16:27:34 +0200	[thread overview]
Message-ID: <20090826142732.GA6304@nowhere> (raw)
In-Reply-To: <20090826141052.GB5946@osiris.boeblingen.de.ibm.com>

On Wed, Aug 26, 2009 at 04:10:52PM +0200, Heiko Carstens wrote:
> On Wed, Aug 26, 2009 at 03:30:22PM +0200, Frederic Weisbecker wrote:
> > On Wed, Aug 26, 2009 at 02:59:43PM +0200, Heiko Carstens wrote:
> > > > Yeah, and as told before, syscalls tracing from kernel thread is
> > > > an interesting point but we can't do it that way.
> > > > 
> > > > I'm queuing this patch for .32, but I need you Signed-off-by to apply it :)
> > > 
> > > That won't always work as pointed out in the other example:
> > > - Process doing sys_init_module then scheduled away
> > > - User enables syscall tracing -> TIF_SYSCALL_FTRACE gets set
> > > - init function of the module gets called and is doing kernel_thread()
> > >   (old API) -> kernel thread inherits TIF_SYSCALL_FTRACE.
> > > 
> > > I don't think that's what you want. You might want to clear the flag for
> > > new processes during fork (only for kernel threads I would guess).
> > > 
> > > At least the current patch leaves a hole.
> > 
> > Ah, there are callsites that use kernel_thread() directly?
> > Does it means that t->mm could be non NULL for such resulting
> > kernel threads, in that case it would be hard to hook on
> > do_fork() to check that.
> 
> Oh yes, you are right. kernel threads created with kernel_thread()
> have t->mm != NULL if the forking process has an mm too.
> 
> There are very few callsites left which still use kernel_thread().
> (the last one in s390 driver code will be gone after the next
> merge window).
> 
> As far as I can there are only four callsites left
> (excluding staging): jffs2 and three in net/bluetooth/*

In that case, I'd suggest to pick the patch that checks for
kernel threads while setting the TIF_FLAGS.

The check for invalid syscall numbers in another patch should be
sufficient to not crash.

We might have rare inaccurate result because of the remaining
buggy callsites but instead of working around on it, these
should be fixed.