This is a follow on patch related to the earlier
discussion about restricting the
spawning of kernel threads. See https://lkml.org/lkml/2013/9/5/426
usermodehelper() threads can currently run on all processors.
This is an issue for low latency cores.
Restrict usermodehelper() threads to the cores that kthreadd is
restricted to.
The default for kthreadd is to be allowed to run on an processors.
If the user restricts kthreadd then threads spawned by
usermodhelper() will similarly restricted.
Before this patch there is no way to limit the cpus that usermodehelper
can run on since the affinity is set when the thread is spawned to
all processors.
Signed-off-by: Christoph Lameter <[email protected]>
Index: linux/include/linux/kthread.h
===================================================================
--- linux.orig/include/linux/kthread.h 2013-10-10 11:00:34.167771996 -0500
+++ linux/include/linux/kthread.h 2013-10-15 13:57:52.859056676 -0500
@@ -51,6 +51,7 @@ void kthread_parkme(void);
int kthreadd(void *unused);
extern struct task_struct *kthreadd_task;
extern int tsk_fork_get_node(struct task_struct *tsk);
+void set_kthreadd_affinity(void);
/*
* Simple work processor based on kthread.
Index: linux/kernel/kmod.c
===================================================================
--- linux.orig/kernel/kmod.c 2013-10-10 11:00:39.091771917 -0500
+++ linux/kernel/kmod.c 2013-10-15 14:02:01.904261324 -0500
@@ -40,6 +40,7 @@
#include <linux/ptrace.h>
#include <linux/async.h>
#include <asm/uaccess.h>
+#include <linux/kthread.h>
#include <trace/events/module.h>
@@ -209,8 +210,8 @@ static int ____call_usermodehelper(void
flush_signal_handlers(current, 1);
spin_unlock_irq(¤t->sighand->siglock);
- /* We can run anywhere, unlike our parent keventd(). */
- set_cpus_allowed_ptr(current, cpu_all_mask);
+ /* We can run anywhere kthreadd can run */
+ set_kthreadd_affinity();
/*
* Our parent is keventd, which runs with elevated scheduling priority.
Index: linux/kernel/kthread.c
===================================================================
--- linux.orig/kernel/kthread.c 2013-10-10 11:00:39.139771916 -0500
+++ linux/kernel/kthread.c 2013-10-15 14:04:36.636043744 -0500
@@ -136,6 +136,15 @@ void *kthread_data(struct task_struct *t
return to_kthread(task)->data;
}
+/*
+ * Set the affinity of the calling task to be the same
+ * as the kthreadd affinities.
+ */
+void set_kthreadd_affinity(void)
+{
+ set_cpus_allowed(current, kthreadd_task->cpus_allowed);
+}
+
/**
* probe_kthread_data - speculative version of kthread_data()
* @task: possible kthread task in question
On Wed, 16 Oct 2013 14:44:28 +0000 Christoph Lameter <[email protected]> wrote:
> This is a follow on patch related to the earlier
> discussion about restricting the
> spawning of kernel threads. See https://lkml.org/lkml/2013/9/5/426
>
>
>
>
> usermodehelper() threads can currently run on all processors.
> This is an issue for low latency cores.
How much of an issue? The severity of the problem is utterly unclear
from this description.
> Restrict usermodehelper() threads to the cores that kthreadd is
> restricted to.
>
> The default for kthreadd is to be allowed to run on an processors.
> If the user restricts kthreadd then threads spawned by
> usermodhelper() will similarly restricted.
>
> Before this patch there is no way to limit the cpus that usermodehelper
> can run on since the affinity is set when the thread is spawned to
> all processors.
>
> ...
>
> --- linux.orig/include/linux/kthread.h 2013-10-10 11:00:34.167771996 -0500
> +++ linux/include/linux/kthread.h 2013-10-15 13:57:52.859056676 -0500
> @@ -51,6 +51,7 @@ void kthread_parkme(void);
> int kthreadd(void *unused);
> extern struct task_struct *kthreadd_task;
> extern int tsk_fork_get_node(struct task_struct *tsk);
> +void set_kthreadd_affinity(void);
>
> /*
> * Simple work processor based on kthread.
> Index: linux/kernel/kmod.c
> ===================================================================
> --- linux.orig/kernel/kmod.c 2013-10-10 11:00:39.091771917 -0500
> +++ linux/kernel/kmod.c 2013-10-15 14:02:01.904261324 -0500
> @@ -40,6 +40,7 @@
> #include <linux/ptrace.h>
> #include <linux/async.h>
> #include <asm/uaccess.h>
> +#include <linux/kthread.h>
>
> #include <trace/events/module.h>
>
> @@ -209,8 +210,8 @@ static int ____call_usermodehelper(void
> flush_signal_handlers(current, 1);
> spin_unlock_irq(¤t->sighand->siglock);
>
> - /* We can run anywhere, unlike our parent keventd(). */
> - set_cpus_allowed_ptr(current, cpu_all_mask);
> + /* We can run anywhere kthreadd can run */
This is a poor comment - it explains "what" (which was utterly obvious)
but doesn't explain "why". The reader will want to know *why*
call_usermodehelper() only runs on kthreadd CPUs, but we didn't tell
him.
> + set_kthreadd_affinity();
>
On Wed, 16 Oct 2013, Andrew Morton wrote:
> > usermodehelper() threads can currently run on all processors.
> > This is an issue for low latency cores.
>
> How much of an issue? The severity of the problem is utterly unclear
> from this description.
This causes a context switch and processing to occur. Can take up to
microseconds which is a pretty long timeframe when decisions have to be
made in microseconds by active processing. This will cause a trade to
fail.
> > - /* We can run anywhere, unlike our parent keventd(). */
> > - set_cpus_allowed_ptr(current, cpu_all_mask);
> > + /* We can run anywhere kthreadd can run */
>
> This is a poor comment - it explains "what" (which was utterly obvious)
> but doesn't explain "why". The reader will want to know *why*
> call_usermodehelper() only runs on kthreadd CPUs, but we didn't tell
> him.
We'd like to have the ability to avoid running usermodehelper on certain
cpus to avoid cpu holdoff situations? Would that we an acceptable
explanation?
Or restricting kthreadd will also restrict usermodehelper spawning to
allow control for all spawned kernel threads?
On Wed, Oct 16, 2013 at 02:44:28PM +0000, Christoph Lameter wrote:
> This is a follow on patch related to the earlier
> discussion about restricting the
> spawning of kernel threads. See https://lkml.org/lkml/2013/9/5/426
>
>
>
>
> usermodehelper() threads can currently run on all processors.
> This is an issue for low latency cores.
This might be redundant with akpm's reply but here we go:
low latency is a very confusing term nowadays. It's used by real time
to describe the time it takes for a high prio task to take the CPU,
and it's used by you for extreme HPC mode for deterministic undisturbed
throughput.
The changelog needs to be expanded to describe what's needed, or people
will be confused when they will stare at that git blame line in one year from now :)
>
> Restrict usermodehelper() threads to the cores that kthreadd is
> restricted to.
>
> The default for kthreadd is to be allowed to run on an processors.
s/an/all ?
> If the user restricts kthreadd then threads spawned by
> usermodhelper() will similarly restricted.
>
> Before this patch there is no way to limit the cpus that usermodehelper
> can run on since the affinity is set when the thread is spawned to
> all processors.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> Index: linux/include/linux/kthread.h
> ===================================================================
> --- linux.orig/include/linux/kthread.h 2013-10-10 11:00:34.167771996 -0500
> +++ linux/include/linux/kthread.h 2013-10-15 13:57:52.859056676 -0500
> @@ -51,6 +51,7 @@ void kthread_parkme(void);
> int kthreadd(void *unused);
> extern struct task_struct *kthreadd_task;
> extern int tsk_fork_get_node(struct task_struct *tsk);
> +void set_kthreadd_affinity(void);
>
> /*
> * Simple work processor based on kthread.
> Index: linux/kernel/kmod.c
> ===================================================================
> --- linux.orig/kernel/kmod.c 2013-10-10 11:00:39.091771917 -0500
> +++ linux/kernel/kmod.c 2013-10-15 14:02:01.904261324 -0500
> @@ -40,6 +40,7 @@
> #include <linux/ptrace.h>
> #include <linux/async.h>
> #include <asm/uaccess.h>
> +#include <linux/kthread.h>
>
> #include <trace/events/module.h>
>
> @@ -209,8 +210,8 @@ static int ____call_usermodehelper(void
> flush_signal_handlers(current, 1);
> spin_unlock_irq(¤t->sighand->siglock);
>
> - /* We can run anywhere, unlike our parent keventd(). */
> - set_cpus_allowed_ptr(current, cpu_all_mask);
> + /* We can run anywhere kthreadd can run */
> + set_kthreadd_affinity();
If you really want your isolated CPUs to be undisturbed, you need this affinity to
be set before the creation of this usermode helper. Otherwise it's going to run to any random
place before you call the above function.
Is there now way to control the kworker affinity which create this usermode helpers threads
instead?
On Thu, 17 Oct 2013, Frederic Weisbecker wrote:
> > - /* We can run anywhere, unlike our parent keventd(). */
> > - set_cpus_allowed_ptr(current, cpu_all_mask);
> > + /* We can run anywhere kthreadd can run */
> > + set_kthreadd_affinity();
>
> If you really want your isolated CPUs to be undisturbed, you need this affinity to
> be set before the creation of this usermode helper. Otherwise it's going to run to any random
> place before you call the above function.
>
> Is there now way to control the kworker affinity which create this usermode helpers threads
> instead?
The invocation is coming from keventd so the affinity seems to be set to
a different cpu before we get the thread moved.
A tsc loop is running on the processor that gets hit by the usermodehelper spawns.
We see spawning by the tty notifier (when typing on a vt) as well as by a
mellanox driver (periodically).
On Thu, Oct 17, 2013 at 03:24:36PM +0000, Christoph Lameter wrote:
> On Thu, 17 Oct 2013, Frederic Weisbecker wrote:
>
> > > - /* We can run anywhere, unlike our parent keventd(). */
> > > - set_cpus_allowed_ptr(current, cpu_all_mask);
> > > + /* We can run anywhere kthreadd can run */
> > > + set_kthreadd_affinity();
> >
> > If you really want your isolated CPUs to be undisturbed, you need this affinity to
> > be set before the creation of this usermode helper. Otherwise it's going to run to any random
> > place before you call the above function.
> >
> > Is there now way to control the kworker affinity which create this usermode helpers threads
> > instead?
>
> The invocation is coming from keventd so the affinity seems to be set to
> a different cpu before we get the thread moved.
Looking at alloc_singlethread_workqueue(), it seems to create a kthread by using
kthread_create(). So I guess the resulting works have the affinity of that kthread.
And thus call_usermodehelper() initially has that affinity, until it spreads it explicitly
to all CPUs.
Couldn't we instead make kthread children (those created with kthread_create()) to inherit
kthread initial affinity? Currently kthread's children have cpu_all_mask. We could change
that behaviour. This way the initial kthread affinity could be inherited all along.
>
> A tsc loop is running on the processor that gets hit by the usermodehelper spawns.
>
> We see spawning by the tty notifier (when typing on a vt) as well as by a
> mellanox driver (periodically).>
On Thu, 17 Oct 2013 18:07:28 +0200 Frederic Weisbecker <[email protected]> wrote:
> Couldn't we instead make kthread children (those created with kthread_create()) to inherit
> kthread initial affinity? Currently kthread's children have cpu_all_mask. We could change
> that behaviour. This way the initial kthread affinity could be inherited all along.
I'm wondering if it's clean/logical to tie usermodehelper affinity to
kthreadd affinity at all. It's certainly convenient, but they're
distinct concepts. What is the reason to not have a separate control
for usermodehelper cpus-allowed?
On Thu, 17 Oct 2013, Andrew Morton wrote:
> On Thu, 17 Oct 2013 18:07:28 +0200 Frederic Weisbecker <[email protected]> wrote:
>
> > Couldn't we instead make kthread children (those created with kthread_create()) to inherit
> > kthread initial affinity? Currently kthread's children have cpu_all_mask. We could change
> > that behaviour. This way the initial kthread affinity could be inherited all along.
>
> I'm wondering if it's clean/logical to tie usermodehelper affinity to
> kthreadd affinity at all. It's certainly convenient, but they're
> distinct concepts. What is the reason to not have a separate control
> for usermodehelper cpus-allowed?
It was suggested in the earlier discussion. We could do it separately.
We have to limit kthreadd anyways and the idea was that the setting would
be used to limit any threads spawned that can be limited to a set of
processors.
On Thu, 17 Oct 2013, Frederic Weisbecker wrote:
> > The invocation is coming from keventd so the affinity seems to be set to
> > a different cpu before we get the thread moved.
>
> Looking at alloc_singlethread_workqueue(), it seems to create a kthread by using
> kthread_create(). So I guess the resulting works have the affinity of that kthread.
> And thus call_usermodehelper() initially has that affinity, until it spreads it explicitly
> to all CPUs.
>
> Couldn't we instead make kthread children (those created with kthread_create()) to inherit
> kthread initial affinity? Currently kthread's children have cpu_all_mask. We could change
> that behaviour. This way the initial kthread affinity could be inherited all along.
Some of the code relies on being able to set the affinities separately.
On Wed, 16 Oct 2013 22:37:52 +0000 Christoph Lameter <[email protected]> wrote:
> > > - /* We can run anywhere, unlike our parent keventd(). */
> > > - set_cpus_allowed_ptr(current, cpu_all_mask);
> > > + /* We can run anywhere kthreadd can run */
> >
> > This is a poor comment - it explains "what" (which was utterly obvious)
> > but doesn't explain "why". The reader will want to know *why*
> > call_usermodehelper() only runs on kthreadd CPUs, but we didn't tell
> > him.
>
> We'd like to have the ability to avoid running usermodehelper on certain
> cpus to avoid cpu holdoff situations? Would that we an acceptable
> explanation?
>
> Or restricting kthreadd will also restrict usermodehelper spawning to
> allow control for all spawned kernel threads?
Both, please ;)
On Thu, Oct 17, 2013 at 10:50:26AM -0700, Andrew Morton wrote:
> On Thu, 17 Oct 2013 18:07:28 +0200 Frederic Weisbecker <[email protected]> wrote:
>
> > Couldn't we instead make kthread children (those created with kthread_create()) to inherit
> > kthread initial affinity? Currently kthread's children have cpu_all_mask. We could change
> > that behaviour. This way the initial kthread affinity could be inherited all along.
>
> I'm wondering if it's clean/logical to tie usermodehelper affinity to
> kthreadd affinity at all. It's certainly convenient, but they're
> distinct concepts. What is the reason to not have a separate control
> for usermodehelper cpus-allowed?
Makes sense yeah. In fact what I'm mostly concerned about is that we should
set the affinity of __call_usermodehelper threads through inheritance from
a parent rather than making it setting its affinity itself. Because in the latter case,
the usermodehelper thread can run anywhere until it sets its affinity. Whether
this little window of global affinity is short or not, this defeats the initial purpose
of this patch that is about isolating CPUs and having them undisturbed.
May be we can do that by setting the affinity of the "khelper" workqueue?
On Fri, 18 Oct 2013, Frederic Weisbecker wrote:
> Makes sense yeah. In fact what I'm mostly concerned about is that we should
> set the affinity of __call_usermodehelper threads through inheritance from
> a parent rather than making it setting its affinity itself. Because in the latter case,
> the usermodehelper thread can run anywhere until it sets its affinity. Whether
> this little window of global affinity is short or not, this defeats the initial purpose
> of this patch that is about isolating CPUs and having them undisturbed.
>
> May be we can do that by setting the affinity of the "khelper" workqueue?
The "parent" for usemodehelper in this case is keventd. The worker queue
item is triggered on a particular processor and thats fine because that is
the result of an OS action or a device irq action. These can already be
avoided. What is not ok is that a process makes a move onto a hardware
thread where we want to have the least OS holdoffs possible.
Setting it via khelper would be fine. Any objections?