public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Subject: Re: [PATCH RESEND 2/2] tracing: syscall_regfunc() should not skip kernel threads
Date: Thu, 10 Apr 2014 19:57:05 +0200	[thread overview]
Message-ID: <20140410175705.GB32332@redhat.com> (raw)
In-Reply-To: <20140410110848.64c3f25e@gandalf.local.home>

On 04/10, Steven Rostedt wrote:
>
> On Thu, 10 Apr 2014 16:46:55 +0200
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > > void tracepoint_check_syscalls(void)
> > > {
> > > 	if (!sys_tracepoint_refcount)
> > > 		return;
> > >
> > > 	read_lock(&tasklist_lock);
> > > 	/* Make sure it wasn't cleared since taking the lock */
> > > 	if (sys_tracepoint_refcount)
> > > 		set_tsk_thread_flag(current, TIF_SYSCALL_TRACEPOINT);
> > > 	read_unlock(&tasklist_lock);
> > > }
> >
> > And how this can help to avoid the race? We need write_lock_irq().
>
> But you chopped off the last part. Where I replaced tasklist_lock with
> a tracepoint specific lock that would synchronize
> sys_tracepoint_refcount with the setting of the flags.

Yes sure, if we add another lock everything is fine.

> > Perhaps I missed something... and I simply do not understand why do you
> > want to do this.
>
> Because I'm being an ass ;-)

Nothing new, I always knew this ;)

> The real reason I'm doing this debate is more to find out exactly what
> the problems are. A learning exercise if you will. I just don't want to
> add a regression, as Hendrik (which I just Cc'd) added the commit for a
> reason. Perhaps you are correct that we should just go back to the way
> things were.

Sure, this should be verified. Besides, the changelog is very old. It says
"kernel_execve() itself does "int 80" on X86_32.", this is no longer true.

> Hendrik, we are debating about removing
> cc3b13c11c567c69a6356be98d0c03ff11541d5c as it stops
> call_usermodehelper tasks from tracing their syscalls.
>
> If Hendrik has no problems with this, neither do I.

OK.

cc3b13c11c567 mentions ret_from_fork, today copy_thread(PF_KTHREAD) uses
ret_from_kernel_thread on 32bit, and still ret_from_fork on 64 bit but
in the last case it checks PF_KTHREAD... I am wondering why they both
(ret_from_kernel_thread and "1: " label in ret_from_fork) can't simply
call do_exit() at the end?

And, since they do not, every kernel_thread's function (fn argument of
kernel_thread) must call do_exit itself?

Looks a bit strange, I guess I missed something obvious.

Oleg.


  reply	other threads:[~2014-04-10 17:57 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-09 16:11 [PATCH 0/5] core: Convert thread iteration to use for_each[_process]_thread APIs, 1st pile Frederic Weisbecker
2014-04-09 16:11 ` [PATCH 1/5] sched: Convert thread_group_cputime() to use for_each_thread() Frederic Weisbecker
2014-04-09 17:12   ` Oleg Nesterov
2014-04-09 17:16   ` Peter Zijlstra
2014-04-09 17:32     ` Oleg Nesterov
2014-04-09 18:30       ` Peter Zijlstra
2014-04-09 19:46         ` Oleg Nesterov
2014-04-09 19:49           ` Peter Zijlstra
2014-04-10 16:19             ` Peter Zijlstra
2014-04-10 16:32               ` Peter Zijlstra
2014-04-10 17:29               ` Oleg Nesterov
2014-04-10 17:36                 ` Peter Zijlstra
2014-04-10 17:42                   ` Peter Zijlstra
2014-04-10 19:15                   ` Oleg Nesterov
2014-04-10 20:55                     ` Peter Zijlstra
2014-04-10  7:56           ` Ingo Molnar
2014-04-09 16:11 ` [PATCH 2/5] tracepoint: Convert process iteration to use for_each_process_thread() Frederic Weisbecker
2014-04-09 16:28   ` Mathieu Desnoyers
2014-04-09 16:40     ` Frederic Weisbecker
2014-04-09 16:42     ` Steven Rostedt
2014-04-09 17:05       ` [PATCH 0/2] Was: " Oleg Nesterov
2014-04-09 17:05         ` [PATCH RESEND 1/2] tracing: syscall_*regfunc() can race with copy_process() Oleg Nesterov
2014-04-10 13:04           ` Steven Rostedt
2014-04-10 13:33             ` Oleg Nesterov
2014-04-10 13:06           ` Steven Rostedt
2014-04-10 13:34             ` Oleg Nesterov
2014-04-11 15:22               ` Steven Rostedt
2014-04-11 15:58                 ` Oleg Nesterov
2014-04-13 18:58                   ` [PATCH v2 0/3] tracing: syscall_*regfunc() fixes Oleg Nesterov
2014-04-13 18:58                     ` [PATCH v2 1/3] tracing: fix syscall_*regfunc() vs copy_process() race Oleg Nesterov
2014-04-14 23:57                       ` Frederic Weisbecker
2014-04-13 18:59                     ` [PATCH v2 2/3] tracing: change syscall_*regfunc() to check PF_KTHREAD and use for_each_process_thread() Oleg Nesterov
2014-04-13 18:59                     ` [PATCH v2 3/3] tracing: syscall_regfunc() should not skip kernel threads Oleg Nesterov
2014-04-14 23:46                     ` [PATCH v2 0/3] tracing: syscall_*regfunc() fixes Frederic Weisbecker
2014-06-18 14:23                     ` Steven Rostedt
2014-06-18 15:36                       ` Oleg Nesterov
2014-04-09 17:06         ` [PATCH RESEND 2/2] tracing: syscall_regfunc() should not skip kernel threads Oleg Nesterov
2014-04-10 13:28           ` Steven Rostedt
2014-04-10 13:38             ` Oleg Nesterov
2014-04-10 14:28               ` Steven Rostedt
2014-04-10 14:46                 ` Oleg Nesterov
2014-04-10 15:08                   ` Steven Rostedt
2014-04-10 17:57                     ` Oleg Nesterov [this message]
2014-04-10 18:14                       ` Oleg Nesterov
2014-04-10 19:00                         ` Oleg Nesterov
2014-04-10 19:13                         ` Steven Rostedt
2014-04-10 19:38                           ` Oleg Nesterov
2014-04-10 19:55                             ` Steven Rostedt
2014-04-11 12:03                               ` Oleg Nesterov
2014-04-11 12:37                                 ` Steven Rostedt
2014-04-10 13:03         ` [PATCH 0/2] Was: Convert process iteration to use for_each_process_thread() Steven Rostedt
2014-04-09 16:11 ` [PATCH 3/5] hung_task: " Frederic Weisbecker
2014-04-09 17:23   ` Oleg Nesterov
2014-04-09 16:11 ` [PATCH 4/5] procfs: Convert process iteration to use for_each_thread() Frederic Weisbecker
2014-04-09 16:11 ` [PATCH 5/5] sched: Convert tasks iteration to use for_each_process_thread() Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140410175705.GB32332@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=brueckner@linux.vnet.ibm.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox