mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>, Christoph Lameter <cl@linux.com>,
	Rik van Riel <riel@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: [PATCH 2/3] kmod: Add up-to-date explanations on the purpose of each asynchronous levels
Date: Mon,  6 Jul 2015 17:33:40 +0200	[thread overview]
Message-ID: <1436196821-13962-3-git-send-email-fweisbec@gmail.com> (raw)
In-Reply-To: <1436196821-13962-1-git-send-email-fweisbec@gmail.com>

There seem to be quite some confusions on the comments, likely due to
changes that came after them.

Now since it's very non obvious why we have 3 levels of asynchronous
code to implement usermodehelpers, it's important to comment in detail
the reason of this layout.

Cc: Rik van Riel <riel@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Lameter <cl@linux.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/kmod.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 4682e91..f940b21 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -269,7 +269,11 @@ out:
 	do_exit(0);
 }
 
-/* Keventd can't block, but this (a child) can. */
+/*
+ * We couldn't wait for usermodehelper completion from khelper without
+ * blocking other pending concurrent usermodehelper targets. This is why
+ * the UMH_WAIT_PROC flavour runs in its own thread.
+ */
 static int call_usermodehelper_exec_sync(void *data)
 {
 	struct subprocess_info *sub_info = data;
@@ -285,8 +289,8 @@ static int call_usermodehelper_exec_sync(void *data)
 		/*
 		 * Normally it is bogus to call wait4() from in-kernel because
 		 * wait4() wants to write the exit code to a userspace address.
-		 * But call_usermodehelper_exec_sync() always runs as keventd,
-		 * and put_user() to a kernel address works OK for kernel
+		 * But call_usermodehelper_exec_sync() always runs as kernel
+		 * thread and put_user() to a kernel address works OK for kernel
 		 * threads, due to their having an mm_segment_t which spans the
 		 * entire address space.
 		 *
@@ -307,7 +311,15 @@ static int call_usermodehelper_exec_sync(void *data)
 	do_exit(0);
 }
 
-/* This is run by khelper thread  */
+/*
+ * This function doesn't need to be called asynchronously. But we need to create
+ * the usermodehelper kernel threads from a task that is affine to all CPUs
+ * (or nohz housekeeping ones) such that they inherit a global affinity. Khelper
+ * workqueue simply provides that.
+ * call_usermodehelper() can be called from tasks with a reduced CPU
+ * affinity (eg: per-cpu workqueues) and we don't want usermodehelper targets to
+ * contend any busy CPU.
+ */
 static void call_usermodehelper_exec_work(struct work_struct *work)
 {
 	struct subprocess_info *sub_info =
@@ -693,6 +705,18 @@ struct ctl_table usermodehelper_table[] = {
 
 void __init usermodehelper_init(void)
 {
+	/*
+	 * The singlethread property here stands for the need of a workqueue
+	 * with wide CPUs affinity, in order to create usermodehelper kernel
+	 * threads inheriting this attribute irrespective of
+	 * call_usermodehelper() callers. Non-singlethread workqueues are
+	 * otherwise per-cpu and wouldn't produce the desired effect.
+	 *
+	 * The ordering guarantee as a side-effect isn't necessary but shouldn't
+	 * introduce performance issue. All we do is creating two kernel threads.
+	 * This should be fast enough not to block concurrent usermodehelper
+	 * callers.
+	 */
 	khelper_wq = create_singlethread_workqueue("khelper");
 	BUG_ON(!khelper_wq);
 }
-- 
2.1.4


  parent reply	other threads:[~2015-07-06 15:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-06 15:33 [PATCH 0/3] kmod: Make nohz-full friendly + cleanups Frederic Weisbecker
2015-07-06 15:33 ` [PATCH 1/3] kmod: Bunch of internal functions renames Frederic Weisbecker
2015-07-06 15:33 ` Frederic Weisbecker [this message]
2015-07-07 23:07   ` [PATCH 2/3] kmod: Add up-to-date explanations on the purpose of each asynchronous levels Andrew Morton
2015-07-08 12:54     ` Frederic Weisbecker
2015-07-06 15:33 ` [PATCH 3/3] kmod: Remove unecessary explicit wide CPU affinity setting Frederic Weisbecker
2015-07-07 16:30   ` Oleg Nesterov
2015-07-07 17:10     ` Frederic Weisbecker
2015-07-07 17:36       ` Oleg Nesterov
2015-07-07 17:38         ` Oleg Nesterov

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=1436196821-13962-3-git-send-email-fweisbec@gmail.com \
    --to=fweisbec@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=riel@redhat.com \
    /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

Powered by JetHome