* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads [not found] <20130317182834.GA22989@redhat.com> @ 2013-03-17 18:54 ` Steven Rostedt 2013-03-17 19:04 ` Oleg Nesterov 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2013-03-17 18:54 UTC (permalink / raw) To: Oleg Nesterov Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On Sun, 2013-03-17 at 19:28 +0100, Oleg Nesterov wrote: > syscall_regfunc() ignores the kernel thread because "it has > no effect", see cc3b13c1 "Don't trace kernel thread syscalls". > > However, this means that a user-space task spawned by > call_usermodehelper() won't report the system calls if > kernel_execve() is called when sys_tracepoint_refcount != 0. > > Remove this check. Hopefully the unnecessary report from > ret_from_fork path mentioned by cc3b13c1 is fine. In fact > "this is the only case" is not true. Say, kernel_execve() > itself does "int 80" on X86_32. Hopefully fine too. > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit ridiculous. We really should have a "swap syscall table when tracepoints enabled" that changes the syscall table that does exactly the same thing as the normal table but wraps the system call with the tracepoints. Something that we are looking to do with interrupts. Altough this may not be that trivial, as this seems to be the method to trace system calls on not only x86, but also PowerPC, ARM, s390, Sparc, and sh. -- Steve > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > kernel/tracepoint.c | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c > index a16754b..4e1e4ca 100644 > --- a/kernel/tracepoint.c > +++ b/kernel/tracepoint.c > @@ -737,9 +737,7 @@ void syscall_regfunc(void) > if (!sys_tracepoint_refcount) { > read_lock(&tasklist_lock); > do_each_thread(g, t) { > - /* Skip kernel threads. */ > - if (!(t->flags & PF_KTHREAD)) > - set_tsk_thread_flag(t, TIF_SYSCALL_TRACEPOINT); > + set_tsk_thread_flag(t, TIF_SYSCALL_TRACEPOINT); > } while_each_thread(g, t); > read_unlock(&tasklist_lock); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-17 18:54 ` [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads Steven Rostedt @ 2013-03-17 19:04 ` Oleg Nesterov 2013-03-17 19:04 ` Oleg Nesterov ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Oleg Nesterov @ 2013-03-17 19:04 UTC (permalink / raw) To: Steven Rostedt Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On 03/17, Steven Rostedt wrote: > > On Sun, 2013-03-17 at 19:28 +0100, Oleg Nesterov wrote: > > syscall_regfunc() ignores the kernel thread because "it has > > no effect", see cc3b13c1 "Don't trace kernel thread syscalls". > > > > However, this means that a user-space task spawned by > > call_usermodehelper() won't report the system calls if > > kernel_execve() is called when sys_tracepoint_refcount != 0. > > > > Remove this check. Hopefully the unnecessary report from > > ret_from_fork path mentioned by cc3b13c1 is fine. In fact > > "this is the only case" is not true. Say, kernel_execve() > > itself does "int 80" on X86_32. Hopefully fine too. > > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit > ridiculous. We really should have a "swap syscall table when tracepoints > enabled" that changes the syscall table that does exactly the same thing > as the normal table but wraps the system call with the tracepoints. But we also need to force the slow path in system_call... Anyway, do you agree with this change for now? Oleg. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-17 19:04 ` Oleg Nesterov @ 2013-03-17 19:04 ` Oleg Nesterov 2013-03-17 19:36 ` Steven Rostedt 2013-03-19 15:10 ` David Howells 2 siblings, 0 replies; 9+ messages in thread From: Oleg Nesterov @ 2013-03-17 19:04 UTC (permalink / raw) To: Steven Rostedt Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On 03/17, Steven Rostedt wrote: > > On Sun, 2013-03-17 at 19:28 +0100, Oleg Nesterov wrote: > > syscall_regfunc() ignores the kernel thread because "it has > > no effect", see cc3b13c1 "Don't trace kernel thread syscalls". > > > > However, this means that a user-space task spawned by > > call_usermodehelper() won't report the system calls if > > kernel_execve() is called when sys_tracepoint_refcount != 0. > > > > Remove this check. Hopefully the unnecessary report from > > ret_from_fork path mentioned by cc3b13c1 is fine. In fact > > "this is the only case" is not true. Say, kernel_execve() > > itself does "int 80" on X86_32. Hopefully fine too. > > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit > ridiculous. We really should have a "swap syscall table when tracepoints > enabled" that changes the syscall table that does exactly the same thing > as the normal table but wraps the system call with the tracepoints. But we also need to force the slow path in system_call... Anyway, do you agree with this change for now? Oleg. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-17 19:04 ` Oleg Nesterov 2013-03-17 19:04 ` Oleg Nesterov @ 2013-03-17 19:36 ` Steven Rostedt 2013-03-18 16:26 ` Oleg Nesterov 2013-03-19 15:10 ` David Howells 2 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2013-03-17 19:36 UTC (permalink / raw) To: Oleg Nesterov Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On Sun, 2013-03-17 at 20:04 +0100, Oleg Nesterov wrote: > > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit > > ridiculous. We really should have a "swap syscall table when tracepoints > > enabled" that changes the syscall table that does exactly the same thing > > as the normal table but wraps the system call with the tracepoints. > > But we also need to force the slow path in system_call... Why? If we remove the tracepoint from the slowpath and use a table swap, then we wouldn't need to use the slowpath at all. > > Anyway, do you agree with this change for now? Well, if it's solving a bug today sure. But we should really be looking at fixing what's there for the future. -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-17 19:36 ` Steven Rostedt @ 2013-03-18 16:26 ` Oleg Nesterov 2013-03-18 16:26 ` Oleg Nesterov 0 siblings, 1 reply; 9+ messages in thread From: Oleg Nesterov @ 2013-03-18 16:26 UTC (permalink / raw) To: Steven Rostedt Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On 03/17, Steven Rostedt wrote: > > On Sun, 2013-03-17 at 20:04 +0100, Oleg Nesterov wrote: > > > > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit > > > ridiculous. We really should have a "swap syscall table when tracepoints > > > enabled" that changes the syscall table that does exactly the same thing > > > as the normal table but wraps the system call with the tracepoints. > > > > But we also need to force the slow path in system_call... > > Why? If we remove the tracepoint from the slowpath and use a table swap, > then we wouldn't need to use the slowpath at all. Ah, indeed, you are right. > > Anyway, do you agree with this change for now? > > Well, if it's solving a bug today sure. But we should really be looking > at fixing what's there for the future. OK, thanks. Oleg. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-18 16:26 ` Oleg Nesterov @ 2013-03-18 16:26 ` Oleg Nesterov 0 siblings, 0 replies; 9+ messages in thread From: Oleg Nesterov @ 2013-03-18 16:26 UTC (permalink / raw) To: Steven Rostedt Cc: Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On 03/17, Steven Rostedt wrote: > > On Sun, 2013-03-17 at 20:04 +0100, Oleg Nesterov wrote: > > > > I'm really thinking the TIF_SYSCALL_TRACEPOINT flag is getting a bit > > > ridiculous. We really should have a "swap syscall table when tracepoints > > > enabled" that changes the syscall table that does exactly the same thing > > > as the normal table but wraps the system call with the tracepoints. > > > > But we also need to force the slow path in system_call... > > Why? If we remove the tracepoint from the slowpath and use a table swap, > then we wouldn't need to use the slowpath at all. Ah, indeed, you are right. > > Anyway, do you agree with this change for now? > > Well, if it's solving a bug today sure. But we should really be looking > at fixing what's there for the future. OK, thanks. Oleg. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-17 19:04 ` Oleg Nesterov 2013-03-17 19:04 ` Oleg Nesterov 2013-03-17 19:36 ` Steven Rostedt @ 2013-03-19 15:10 ` David Howells 2013-03-19 15:36 ` Steven Rostedt 2 siblings, 1 reply; 9+ messages in thread From: David Howells @ 2013-03-19 15:10 UTC (permalink / raw) To: Steven Rostedt Cc: dhowells, Oleg Nesterov, Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch Steven Rostedt <rostedt@goodmis.org> wrote: > Why? If we remove the tracepoint from the slowpath and use a table swap, > then we wouldn't need to use the slowpath at all. How are you engineering a table swap? Do you patch the system call code to change the immediate address loaded or do you put in a level of indirection? David ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-19 15:10 ` David Howells @ 2013-03-19 15:36 ` Steven Rostedt 2013-03-19 21:27 ` H. Peter Anvin 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2013-03-19 15:36 UTC (permalink / raw) To: David Howells Cc: Oleg Nesterov, Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, H. Peter Anvin, linux-arch On Tue, 2013-03-19 at 15:10 +0000, David Howells wrote: > Steven Rostedt <rostedt@goodmis.org> wrote: > > > Why? If we remove the tracepoint from the slowpath and use a table swap, > > then we wouldn't need to use the slowpath at all. > > How are you engineering a table swap? Do you patch the system call code to > change the immediate address loaded or do you put in a level of indirection? Patching the call site would probably be the easiest method. We've gotten pretty good at doing that ;-) -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads 2013-03-19 15:36 ` Steven Rostedt @ 2013-03-19 21:27 ` H. Peter Anvin 0 siblings, 0 replies; 9+ messages in thread From: H. Peter Anvin @ 2013-03-19 21:27 UTC (permalink / raw) To: Steven Rostedt Cc: David Howells, Oleg Nesterov, Andrew Morton, Ingo Molnar, Frederic Weisbecker, linux-kernel, linux-arch On 03/19/2013 08:36 AM, Steven Rostedt wrote: > On Tue, 2013-03-19 at 15:10 +0000, David Howells wrote: >> Steven Rostedt <rostedt@goodmis.org> wrote: >> >>> Why? If we remove the tracepoint from the slowpath and use a table swap, >>> then we wouldn't need to use the slowpath at all. >> >> How are you engineering a table swap? Do you patch the system call code to >> change the immediate address loaded or do you put in a level of indirection? > > Patching the call site would probably be the easiest method. > > We've gotten pretty good at doing that ;-) > Yes, given that the machinery is already there we might as well use it. -hpa ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-03-19 21:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130317182834.GA22989@redhat.com>
2013-03-17 18:54 ` [PATCH 2/2] tracing: syscall_regfunc() should not skip kernel threads Steven Rostedt
2013-03-17 19:04 ` Oleg Nesterov
2013-03-17 19:04 ` Oleg Nesterov
2013-03-17 19:36 ` Steven Rostedt
2013-03-18 16:26 ` Oleg Nesterov
2013-03-18 16:26 ` Oleg Nesterov
2013-03-19 15:10 ` David Howells
2013-03-19 15:36 ` Steven Rostedt
2013-03-19 21:27 ` H. Peter Anvin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).