All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.