On Fri, 8 Nov 2013 21:06:22 +0100 Frederic Weisbecker <[email protected]> wrote:
> On Fri, Nov 08, 2013 at 07:52:37PM +0000, Christoph Lameter wrote:
> > On Fri, 8 Nov 2013, Frederic Weisbecker wrote:
> >
> > > I understand, but why not solving that from the workqueue affinity? We want to
> > > solve the issue of unbound workqueues in CPU isolation anyway.
> >
> > Sure if you can solve that with an unbound work queue then this patch is
> > not needed. Do you have a patch that addresses this issue in your
> > patchset?
>
> No. Sorry.
>
Several months have passed and nothgin has happened. Any thoughts on
what we should do with Christoph's patch?
From: Christoph Lameter <[email protected]>
Subject: kmod: run usermodehelpers only on cpus allowed for kthreadd V2
usermodehelper() threads can currently run on all processors. This is an
issue for low latency cores. Spawnig a new thread causes cpu holdoffs in
the range of hundreds of microseconds to a few milliseconds. Not good for
cores on which processes run that need to react as fast as possible.
kthreadd threads can be restricted using taskset to a limited set of
processors. Then the kernel thread pool will not fork processes on those
anymore thereby protecting those processors from additional latencies.
Make usermodehelper() threads obey the limitations that kthreadd is
restricted to. Kthreadd is not the parent of usermodehelper threads so we
need to explicitly get the allowed processors for kthreadd.
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.
[[email protected]: set_cpus_allowed() doesn't exist when CONFIG_CPUMASK_OFFSTACK=y]
[[email protected]: coding-style fixes]
Signed-off-by: Christoph Lameter <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Gilad Ben-Yossef <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Mike Frysinger <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Rusty Russell <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
include/linux/kthread.h | 1 +
kernel/kmod.c | 11 +++++++++--
kernel/kthread.c | 9 +++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
diff -puN include/linux/kthread.h~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2 include/linux/kthread.h
--- a/include/linux/kthread.h~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2
+++ a/include/linux/kthread.h
@@ -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.
diff -puN kernel/kmod.c~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2 kernel/kmod.c
--- a/kernel/kmod.c~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2
+++ a/kernel/kmod.c
@@ -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,14 @@ 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);
+ /*
+ * Kthreadd can be restricted to a set of processors if the user wants
+ * to protect other processors from OS latencies. If that has happened
+ * then we do not want to disturb the other processors here either so we
+ * start the usermode helper threads only on the processors allowed for
+ * kthreadd.
+ */
+ set_kthreadd_affinity();
/*
* Our parent is keventd, which runs with elevated scheduling priority.
diff -puN kernel/kthread.c~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2 kernel/kthread.c
--- a/kernel/kthread.c~kmod-run-usermodehelpers-only-on-cpus-allowed-for-kthreadd-v2
+++ a/kernel/kthread.c
@@ -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_ptr(current, &kthreadd_task->cpus_allowed);
+}
+
/**
* probe_kthread_data - speculative version of kthread_data()
* @task: possible kthread task in question
_
On Tue, Apr 08, 2014 at 01:57:12PM -0700, Andrew Morton wrote:
> On Fri, 8 Nov 2013 21:06:22 +0100 Frederic Weisbecker <[email protected]> wrote:
>
> > On Fri, Nov 08, 2013 at 07:52:37PM +0000, Christoph Lameter wrote:
> > > On Fri, 8 Nov 2013, Frederic Weisbecker wrote:
> > >
> > > > I understand, but why not solving that from the workqueue affinity? We want to
> > > > solve the issue of unbound workqueues in CPU isolation anyway.
> > >
> > > Sure if you can solve that with an unbound work queue then this patch is
> > > not needed. Do you have a patch that addresses this issue in your
> > > patchset?
> >
> > No. Sorry.
> >
>
> Several months have passed and nothgin has happened. Any thoughts on
> what we should do with Christoph's patch?
We have taken another direction to solve the initial problem which is
that usermodehelper threads disturb CPUs on some workloads requiring isolation
(ie: workloads that want specific CPUs to not be disturbed by kernel noise)
We eventually agreed that setting the WQ_SYSFS flag to the usermodehelper
workqueue would allow us to control the affinity of these jobs through sysfs.
But eventually I took it one step further by considering that the real problem
is much broader and concerns all unbound workqueues. Not just usermodehelper.
Christoph finds usermodehelper workqueue disturbing on his workloads, but other
CPU isolation users runnning other workloads on other hardware, other IO protocols, etc..
may be disturbed by many other kind of unbound workqueues. Because unbound
workqueues are globally affined and this behaviour can't be tweaked sanely yet.
And there are a big lot of them. Singlethread workqueues for example has more
than 200 users in the kernel:
$ git grep create_singlethread_workqueue | wc -l
219
It's actually something that we wanted to fix for a while now. So I'm
currently working on a more general solution that allow users to affine all
unbound workqueues to the desired set of CPUs.
The last version of the patchset is https://lkml.org/lkml/2014/3/27/246
After some discussion with Tejun and others, I'm working on the next iteration.
So I strongly think we should drop Christoph's patch and focus on the
unbound workqueues sysfs' affinity interface patchset.
Ah there is also the problem of this explicit call in kmod:
"set_cpus_allowed_ptr(current, cpu_all_mask);"
But it looks like something we can drop. The comment above says:
/* We can run anywhere, unlike our parent keventd(). */
Looks like a stale issue that concerned the old workqueue times when workqueues
really mapped to named thread. But even then I don't see the reason for this
affinity overwrite.
Thanks.