linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 00/13] support "task_isolation" mode
@ 2016-04-05 17:38 Chris Metcalf
  2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Chris Metcalf @ 2016-04-05 17:38 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, Andy Lutomirski,
	Daniel Lezcano, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Chris Metcalf

Here is a respin of the task-isolation patch set.  The previous one
came out just before the merge window for 4.6 opened, so I suspect
folks may have been busy merging, since it got few comments.

Frederic, how are you feeling about taking this all via your tree?
And what is your take on the new PR_TASK_ISOLATION_ONE_SHOT mode?
I'm not sure what the right path to upstream for this series is.

Changes since v11:

- Rebased on v4.6-rc1.  This required me to create a
  can_stop_my_full_tick() helper in tick-sched.c, since the underlying
  can_stop_full_tick() now takes a struct tick_sched.

- Added a HAVE_ARCH_TASK_ISOLATION Kconfig flag so that you can't
  try to build with TASK_ISOLATION enabled for an architecture until
  it is explicitly configured to work.  This avoids possible
  allyesconfig build failures for unsupported architectures, or even
  for supported ones when bisecting to the middle of this series.

- Return EAGAIN instead of EINVAL for the enabling prctl() if the task
  is affinitized to a task-isolation core, but things just aren't yet
  right for it (e.g. another task running).  This lets the caller
  differentiate a potentially transient failure from a permanent
  failure, for which we still return EINVAL.

The previous (v11) patch series is here:

https://lkml.kernel.org/r/1457734223-26209-1-git-send-email-cmetcalf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org

This version of the patch series has been tested on arm64 and tile,
and build-tested on x86.

It remains true that the 1 Hz tick needs to be disabled for this
patch series to be able to achieve its primary goal of enabling
truly tick-free operation, but that is ongoing orthogonal work.

The series is available at:

  git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git dataplane

Chris Metcalf (13):
  vmstat: add quiet_vmstat_sync function
  vmstat: add vmstat_idle function
  lru_add_drain_all: factor out lru_add_drain_needed
  task_isolation: add initial support
  task_isolation: support CONFIG_TASK_ISOLATION_ALL
  task_isolation: support PR_TASK_ISOLATION_STRICT mode
  task_isolation: add debug boot flag
  task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag
  arm, tile: turn off timer tick for oneshot_stopped state
  arch/x86: enable task isolation functionality
  arch/tile: enable task isolation functionality
  arm64: factor work_pending state machine to C
  arch/arm64: enable task isolation functionality

 Documentation/kernel-parameters.txt    |  16 ++
 arch/arm64/Kconfig                     |   1 +
 arch/arm64/include/asm/thread_info.h   |   5 +-
 arch/arm64/kernel/entry.S              |  12 +-
 arch/arm64/kernel/ptrace.c             |  15 +-
 arch/arm64/kernel/signal.c             |  42 ++++-
 arch/arm64/kernel/smp.c                |   2 +
 arch/arm64/mm/fault.c                  |   4 +
 arch/tile/Kconfig                      |   1 +
 arch/tile/include/asm/thread_info.h    |   4 +-
 arch/tile/kernel/process.c             |   9 +
 arch/tile/kernel/ptrace.c              |   7 +
 arch/tile/kernel/single_step.c         |   5 +
 arch/tile/kernel/smp.c                 |  28 +--
 arch/tile/kernel/time.c                |   1 +
 arch/tile/kernel/unaligned.c           |   3 +
 arch/tile/mm/fault.c                   |   3 +
 arch/tile/mm/homecache.c               |   2 +
 arch/x86/Kconfig                       |   1 +
 arch/x86/entry/common.c                |  18 +-
 arch/x86/include/asm/thread_info.h     |   2 +
 arch/x86/kernel/traps.c                |   2 +
 arch/x86/mm/fault.c                    |   2 +
 drivers/base/cpu.c                     |  18 ++
 drivers/clocksource/arm_arch_timer.c   |   2 +
 include/linux/context_tracking_state.h |   6 +
 include/linux/isolation.h              |  63 +++++++
 include/linux/sched.h                  |   3 +
 include/linux/swap.h                   |   1 +
 include/linux/tick.h                   |   2 +
 include/linux/vmstat.h                 |   4 +
 include/uapi/linux/prctl.h             |   9 +
 init/Kconfig                           |  33 ++++
 kernel/Makefile                        |   1 +
 kernel/fork.c                          |   3 +
 kernel/irq_work.c                      |   5 +-
 kernel/isolation.c                     | 316 +++++++++++++++++++++++++++++++++
 kernel/sched/core.c                    |  18 ++
 kernel/signal.c                        |   8 +
 kernel/smp.c                           |   6 +-
 kernel/softirq.c                       |  33 ++++
 kernel/sys.c                           |   9 +
 kernel/time/tick-sched.c               |  36 ++--
 mm/swap.c                              |  15 +-
 mm/vmstat.c                            |  21 +++
 45 files changed, 743 insertions(+), 54 deletions(-)
 create mode 100644 include/linux/isolation.h
 create mode 100644 kernel/isolation.c

-- 
2.7.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v12 04/13] task_isolation: add initial support
  2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
@ 2016-04-05 17:38 ` Chris Metcalf
  2016-05-18 13:34   ` Peter Zijlstra
  2016-04-05 17:38 ` [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Chris Metcalf @ 2016-04-05 17:38 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, Andy Lutomirski,
	Michal Hocko, linux-mm, linux-doc, linux-api, linux-kernel
  Cc: Chris Metcalf

The existing nohz_full mode is designed as a "soft" isolation mode
that makes tradeoffs to minimize userspace interruptions while
still attempting to avoid overheads in the kernel entry/exit path,
to provide 100% kernel semantics, etc.

However, some applications require a "hard" commitment from the
kernel to avoid interruptions, in particular userspace device driver
style applications, such as high-speed networking code.

This change introduces a framework to allow applications
to elect to have the "hard" semantics as needed, specifying
prctl(PR_SET_TASK_ISOLATION, PR_TASK_ISOLATION_ENABLE) to do so.
Subsequent commits will add additional flags and additional
semantics.

The kernel must be built with the new TASK_ISOLATION Kconfig flag
to enable this mode, and the kernel booted with an appropriate
task_isolation=CPULIST boot argument, which enables nohz_full and
isolcpus as well.  The "task_isolation" state is then indicated by
setting a new task struct field, task_isolation_flag, to the value
passed by prctl(), and also setting a TIF_TASK_ISOLATION bit in
thread_info flags.  When task isolation is enabled for a task, and it
is returning to userspace on a task isolation core, it calls the
new task_isolation_ready() / task_isolation_enter() routines to
take additional actions to help the task avoid being interrupted
in the future.

A new /sys/devices/system/cpu/task_isolation pseudo-file is added,
parallel to the comparable nohz_full file.

The task_isolation_ready() call is invoked when TIF_TASK_ISOLATION is
set in prepare_exit_to_usermode() or its architectural equivalent,
and forces the loop to retry if the system is not ready.  It is
called with interrupts disabled and inspects the kernel state
to determine if it is safe to return into an isolated state.
In particular, if it sees that the scheduler tick is still enabled,
it reports that it is not yet safe.

Each time through the loop of TIF work to do, if TIF_TASK_ISOLATION
is set, we call the new task_isolation_enter() routine.  This
takes any actions that might avoid a future interrupt to the core,
such as a worker thread being scheduled that could be quiesced now
(e.g. the vmstat worker) or a future IPI to the core to clean up some
state that could be cleaned up now (e.g. the mm lru per-cpu cache).
In addition, it reqeusts rescheduling if the scheduler dyntick is
still running.

As a result of these tests on the "return to userspace" path, sys
calls (and page faults, etc.) can be inordinately slow.  However,
this quiescing guarantees that no unexpected interrupts will occur,
even if the application intentionally calls into the kernel.

Separate patches that follow provide these changes for x86, tile,
and arm64.

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
---
 Documentation/kernel-parameters.txt |   8 ++
 drivers/base/cpu.c                  |  18 +++++
 include/linux/isolation.h           |  48 +++++++++++
 include/linux/sched.h               |   3 +
 include/linux/tick.h                |   2 +
 include/uapi/linux/prctl.h          |   5 ++
 init/Kconfig                        |  23 ++++++
 kernel/Makefile                     |   1 +
 kernel/fork.c                       |   3 +
 kernel/isolation.c                  | 153 ++++++++++++++++++++++++++++++++++++
 kernel/signal.c                     |   4 +
 kernel/sys.c                        |   9 +++
 kernel/time/tick-sched.c            |  36 ++++++---
 13 files changed, 300 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/isolation.h
 create mode 100644 kernel/isolation.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index ecc74fa4bfde..9bd5e91357b1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3808,6 +3808,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			neutralize any effect of /proc/sys/kernel/sysrq.
 			Useful for debugging.
 
+	task_isolation=	[KNL]
+			In kernels built with CONFIG_TASK_ISOLATION=y, set
+			the specified list of CPUs where cpus will be able
+			to use prctl(PR_SET_TASK_ISOLATION) to set up task
+			isolation mode.  Setting this boot flag implicitly
+			also sets up nohz_full and isolcpus mode for the
+			listed set of cpus.
+
 	tcpmhash_entries= [KNL,NET]
 			Set the number of tcp_metrics_hash slots.
 			Default value is 8192 or 16384 depending on total
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 691eeea2f19a..eaf40f4264ee 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/cpufeature.h>
 #include <linux/tick.h>
+#include <linux/isolation.h>
 
 #include "base.h"
 
@@ -290,6 +291,20 @@ static ssize_t print_cpus_nohz_full(struct device *dev,
 static DEVICE_ATTR(nohz_full, 0444, print_cpus_nohz_full, NULL);
 #endif
 
+#ifdef CONFIG_TASK_ISOLATION
+static ssize_t print_cpus_task_isolation(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	int n = 0, len = PAGE_SIZE-2;
+
+	n = scnprintf(buf, len, "%*pbl\n", cpumask_pr_args(task_isolation_map));
+
+	return n;
+}
+static DEVICE_ATTR(task_isolation, 0444, print_cpus_task_isolation, NULL);
+#endif
+
 static void cpu_device_release(struct device *dev)
 {
 	/*
@@ -460,6 +475,9 @@ static struct attribute *cpu_root_attrs[] = {
 #ifdef CONFIG_NO_HZ_FULL
 	&dev_attr_nohz_full.attr,
 #endif
+#ifdef CONFIG_TASK_ISOLATION
+	&dev_attr_task_isolation.attr,
+#endif
 #ifdef CONFIG_GENERIC_CPU_AUTOPROBE
 	&dev_attr_modalias.attr,
 #endif
diff --git a/include/linux/isolation.h b/include/linux/isolation.h
new file mode 100644
index 000000000000..99b909462e64
--- /dev/null
+++ b/include/linux/isolation.h
@@ -0,0 +1,48 @@
+/*
+ * Task isolation related global functions
+ */
+#ifndef _LINUX_ISOLATION_H
+#define _LINUX_ISOLATION_H
+
+#include <linux/tick.h>
+#include <linux/prctl.h>
+
+#ifdef CONFIG_TASK_ISOLATION
+
+/* cpus that are configured to support task isolation */
+extern cpumask_var_t task_isolation_map;
+
+extern int task_isolation_init(void);
+
+static inline bool task_isolation_possible(int cpu)
+{
+	return task_isolation_map != NULL &&
+		cpumask_test_cpu(cpu, task_isolation_map);
+}
+
+extern int task_isolation_set(unsigned int flags);
+
+extern bool task_isolation_ready(void);
+extern void task_isolation_enter(void);
+
+static inline void task_isolation_set_flags(struct task_struct *p,
+					    unsigned int flags)
+{
+	p->task_isolation_flags = flags;
+
+	if (flags & PR_TASK_ISOLATION_ENABLE)
+		set_tsk_thread_flag(p, TIF_TASK_ISOLATION);
+	else
+		clear_tsk_thread_flag(p, TIF_TASK_ISOLATION);
+}
+
+#else
+static inline void task_isolation_init(void) { }
+static inline bool task_isolation_possible(int cpu) { return false; }
+static inline bool task_isolation_ready(void) { return true; }
+static inline void task_isolation_enter(void) { }
+extern inline void task_isolation_set_flags(struct task_struct *p,
+					    unsigned int flags) { }
+#endif
+
+#endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 60bba7e032dc..90f6856493bb 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1852,6 +1852,9 @@ struct task_struct {
 #ifdef CONFIG_MMU
 	struct task_struct *oom_reaper_list;
 #endif
+#ifdef CONFIG_TASK_ISOLATION
+	unsigned int	task_isolation_flags;
+#endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
 /*
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 62be0786d6d0..fbd81e322860 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -235,6 +235,8 @@ static inline void tick_dep_clear_signal(struct signal_struct *signal,
 
 extern void tick_nohz_full_kick_cpu(int cpu);
 extern void __tick_nohz_task_switch(void);
+extern void tick_nohz_full_add_cpus(const struct cpumask *mask);
+extern bool can_stop_my_full_tick(void);
 #else
 static inline int housekeeping_any_cpu(void)
 {
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index a8d0759a9e40..67224df4b559 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -197,4 +197,9 @@ struct prctl_mm_map {
 # define PR_CAP_AMBIENT_LOWER		3
 # define PR_CAP_AMBIENT_CLEAR_ALL	4
 
+/* Enable/disable or query task_isolation mode for NO_HZ_FULL kernels. */
+#define PR_SET_TASK_ISOLATION		48
+#define PR_GET_TASK_ISOLATION		49
+# define PR_TASK_ISOLATION_ENABLE	(1 << 0)
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/init/Kconfig b/init/Kconfig
index e0d26162432e..767f37bc3391 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -782,6 +782,29 @@ config RCU_EXPEDITE_BOOT
 
 endmenu # "RCU Subsystem"
 
+config HAVE_ARCH_TASK_ISOLATION
+	bool
+
+config TASK_ISOLATION
+	bool "Provide hard CPU isolation from the kernel on demand"
+	depends on NO_HZ_FULL && HAVE_ARCH_TASK_ISOLATION
+	help
+	 Allow userspace processes to place themselves on task_isolation
+	 cores and run prctl(PR_SET_TASK_ISOLATION) to "isolate"
+	 themselves from the kernel.  On return to userspace,
+	 isolated tasks will first arrange that no future kernel
+	 activity will interrupt the task while the task is running
+	 in userspace.  This "hard" isolation from the kernel is
+	 required for userspace tasks that are running hard real-time
+	 tasks in userspace, such as a 10 Gbit network driver in userspace.
+
+	 Without this option, but with NO_HZ_FULL enabled, the kernel
+	 will make a best-faith, "soft" effort to shield a single userspace
+	 process from interrupts, but makes no guarantees.
+
+	 You should say "N" unless you are intending to run a
+	 high-performance userspace driver or similar task.
+
 config BUILD_BIN2C
 	bool
 	default n
diff --git a/kernel/Makefile b/kernel/Makefile
index f0c40bf49d9f..5281b866b0a1 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_TORTURE_TEST) += torture.o
 obj-$(CONFIG_MEMBARRIER) += membarrier.o
 
 obj-$(CONFIG_HAS_IOMEM) += memremap.o
+obj-$(CONFIG_TASK_ISOLATION) += isolation.o
 
 $(obj)/configs.o: $(obj)/config_data.h
 
diff --git a/kernel/fork.c b/kernel/fork.c
index d277e83ed3e0..8541b7ee231c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -76,6 +76,7 @@
 #include <linux/compiler.h>
 #include <linux/sysctl.h>
 #include <linux/kcov.h>
+#include <linux/isolation.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1507,6 +1508,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 #endif
 	clear_all_latency_tracing(p);
 
+	task_isolation_set_flags(p, 0);
+
 	/* ok, now we should be set up.. */
 	p->pid = pid_nr(pid);
 	if (clone_flags & CLONE_THREAD) {
diff --git a/kernel/isolation.c b/kernel/isolation.c
new file mode 100644
index 000000000000..282a34ecb22a
--- /dev/null
+++ b/kernel/isolation.c
@@ -0,0 +1,153 @@
+/*
+ *  linux/kernel/isolation.c
+ *
+ *  Implementation for task isolation.
+ *
+ *  Distributed under GPLv2.
+ */
+
+#include <linux/mm.h>
+#include <linux/swap.h>
+#include <linux/vmstat.h>
+#include <linux/isolation.h>
+#include <linux/syscalls.h>
+#include "time/tick-sched.h"
+
+cpumask_var_t task_isolation_map;
+static bool saw_boot_arg;
+
+/*
+ * Isolation requires both nohz and isolcpus support from the scheduler.
+ * We provide a boot flag that enables both for now, and which we can
+ * add other functionality to over time if needed.  Note that just
+ * specifying "nohz_full=... isolcpus=..." does not enable task isolation.
+ */
+static int __init task_isolation_setup(char *str)
+{
+	saw_boot_arg = true;
+
+	alloc_bootmem_cpumask_var(&task_isolation_map);
+	if (cpulist_parse(str, task_isolation_map) < 0) {
+		pr_warn("task_isolation: Incorrect cpumask '%s'\n", str);
+		return 1;
+	}
+
+	return 1;
+}
+__setup("task_isolation=", task_isolation_setup);
+
+int __init task_isolation_init(void)
+{
+	/* For offstack cpumask, ensure we allocate an empty cpumask early. */
+	if (!saw_boot_arg) {
+		zalloc_cpumask_var(&task_isolation_map, GFP_KERNEL);
+		return 0;
+	}
+
+	/*
+	 * Add our task_isolation cpus to nohz_full and isolcpus.  Note
+	 * that we are called relatively early in boot, from tick_init();
+	 * at this point neither nohz_full nor isolcpus has been used
+	 * to configure the system, but isolcpus has been allocated
+	 * already in sched_init().
+	 */
+	tick_nohz_full_add_cpus(task_isolation_map);
+	cpumask_or(cpu_isolated_map, cpu_isolated_map, task_isolation_map);
+
+	return 0;
+}
+
+/*
+ * Get a snapshot of whether, at this moment, it would be possible to
+ * stop the tick.  This test normally requires interrupts disabled since
+ * the condition can change if an interrupt is delivered.  However, in
+ * this case we are using it in an advisory capacity to see if there
+ * is anything obviously indicating that the task isolation
+ * preconditions have not been met, so it's OK that in principle it
+ * might not still be true later in the prctl() syscall path.
+ */
+static bool can_stop_my_full_tick_now(void)
+{
+	bool ret;
+
+	local_irq_disable();
+	ret = can_stop_my_full_tick();
+	local_irq_enable();
+	return ret;
+}
+
+/*
+ * This routine controls whether we can enable task-isolation mode.
+ * The task must be affinitized to a single task_isolation core, or
+ * else we return EINVAL.  And, it must be at least statically able to
+ * stop the nohz_full tick (e.g., no other schedulable tasks currently
+ * running, no POSIX cpu timers currently set up, etc.); if not, we
+ * return EAGAIN.
+ *
+ * Although the application could later re-affinitize to a
+ * housekeeping core and lose task isolation semantics, or other tasks
+ * could be forcibly scheduled onto this core to restart preemptive
+ * scheduling, etc., this initial test should catch 99% of bugs with
+ * task placement prior to enabling task isolation.
+ */
+int task_isolation_set(unsigned int flags)
+{
+	if (flags != 0) {
+		if (cpumask_weight(tsk_cpus_allowed(current)) != 1 ||
+		    !task_isolation_possible(raw_smp_processor_id()))
+			return -EINVAL;
+		if (!can_stop_my_full_tick_now())
+			return -EAGAIN;
+	}
+
+	task_isolation_set_flags(current, flags);
+	return 0;
+}
+
+/*
+ * In task isolation mode we try to return to userspace only after
+ * attempting to make sure we won't be interrupted again.  This test
+ * is run with interrupts disabled to test that everything we need
+ * to be true is true before we can return to userspace.
+ */
+bool task_isolation_ready(void)
+{
+	WARN_ON_ONCE(!irqs_disabled());
+
+	return (!lru_add_drain_needed(smp_processor_id()) &&
+		vmstat_idle() &&
+		tick_nohz_tick_stopped());
+}
+
+/*
+ * Each time we try to prepare for return to userspace in a process
+ * with task isolation enabled, we run this code to quiesce whatever
+ * subsystems we can readily quiesce to avoid later interrupts.
+ */
+void task_isolation_enter(void)
+{
+	WARN_ON_ONCE(irqs_disabled());
+
+	/* Drain the pagevecs to avoid unnecessary IPI flushes later. */
+	lru_add_drain();
+
+	/* Quieten the vmstat worker so it won't interrupt us. */
+	quiet_vmstat_sync();
+
+	/*
+	 * Request rescheduling unless we are in full dynticks mode.
+	 * We would eventually get pre-empted without this, and if
+	 * there's another task waiting, it would run; but by
+	 * explicitly requesting the reschedule, we may reduce the
+	 * latency.  We could directly call schedule() here as well,
+	 * but since our caller is the standard place where schedule()
+	 * is called, we defer to the caller.
+	 *
+	 * A more substantive approach here would be to use a struct
+	 * completion here explicitly, and complete it when we shut
+	 * down dynticks, but since we presumably have nothing better
+	 * to do on this core anyway, just spinning seems plausible.
+	 */
+	if (!tick_nohz_tick_stopped())
+		set_tsk_need_resched(current);
+}
diff --git a/kernel/signal.c b/kernel/signal.c
index aa9bf00749c1..53e4e62f2778 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -34,6 +34,7 @@
 #include <linux/compat.h>
 #include <linux/cn_proc.h>
 #include <linux/compiler.h>
+#include <linux/isolation.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>
@@ -2213,6 +2214,9 @@ relock:
 		/* Trace actually delivered signals. */
 		trace_signal_deliver(signr, &ksig->info, ka);
 
+		/* Disable task isolation when delivering a signal. */
+		task_isolation_set_flags(current, 0);
+
 		if (ka->sa.sa_handler == SIG_IGN) /* Do nothing.  */
 			continue;
 		if (ka->sa.sa_handler != SIG_DFL) {
diff --git a/kernel/sys.c b/kernel/sys.c
index cf8ba545c7d3..6d5b87273fcc 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -41,6 +41,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/version.h>
 #include <linux/ctype.h>
+#include <linux/isolation.h>
 
 #include <linux/compat.h>
 #include <linux/syscalls.h>
@@ -2269,6 +2270,14 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_GET_FP_MODE:
 		error = GET_FP_MODE(me);
 		break;
+#ifdef CONFIG_TASK_ISOLATION
+	case PR_SET_TASK_ISOLATION:
+		error = task_isolation_set(arg2);
+		break;
+	case PR_GET_TASK_ISOLATION:
+		error = me->task_isolation_flags;
+		break;
+#endif
 	default:
 		error = -EINVAL;
 		break;
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 084b79f5917e..04e77e562ea1 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -23,6 +23,7 @@
 #include <linux/irq_work.h>
 #include <linux/posix-timers.h>
 #include <linux/context_tracking.h>
+#include <linux/isolation.h>
 
 #include <asm/irq_regs.h>
 
@@ -207,6 +208,11 @@ static bool can_stop_full_tick(struct tick_sched *ts)
 	return true;
 }
 
+bool can_stop_my_full_tick(void)
+{
+	return can_stop_full_tick(this_cpu_ptr(&tick_cpu_sched));
+}
+
 static void nohz_full_kick_func(struct irq_work *work)
 {
 	/* Empty, the tick restart happens on tick_nohz_irq_exit() */
@@ -408,30 +414,34 @@ static int tick_nohz_cpu_down_callback(struct notifier_block *nfb,
 	return NOTIFY_OK;
 }
 
-static int tick_nohz_init_all(void)
+void tick_nohz_full_add_cpus(const struct cpumask *mask)
 {
-	int err = -1;
+	if (!cpumask_weight(mask))
+		return;
 
-#ifdef CONFIG_NO_HZ_FULL_ALL
-	if (!alloc_cpumask_var(&tick_nohz_full_mask, GFP_KERNEL)) {
+	if (tick_nohz_full_mask == NULL &&
+	    !zalloc_cpumask_var(&tick_nohz_full_mask, GFP_KERNEL)) {
 		WARN(1, "NO_HZ: Can't allocate full dynticks cpumask\n");
-		return err;
+		return;
 	}
-	err = 0;
-	cpumask_setall(tick_nohz_full_mask);
+
+	cpumask_or(tick_nohz_full_mask, tick_nohz_full_mask, mask);
 	tick_nohz_full_running = true;
-#endif
-	return err;
 }
 
 void __init tick_nohz_init(void)
 {
 	int cpu;
 
-	if (!tick_nohz_full_running) {
-		if (tick_nohz_init_all() < 0)
-			return;
-	}
+	task_isolation_init();
+
+#ifdef CONFIG_NO_HZ_FULL_ALL
+	if (!tick_nohz_full_running)
+		tick_nohz_full_add_cpus(cpu_possible_mask);
+#endif
+
+	if (!tick_nohz_full_running)
+		return;
 
 	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
 		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-- 
2.7.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode
  2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
  2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
@ 2016-04-05 17:38 ` Chris Metcalf
       [not found]   ` <1459877922-15512-7-git-send-email-cmetcalf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
  2016-04-05 17:38 ` [PATCH v12 08/13] task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag Chris Metcalf
  2016-05-12 18:26 ` [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
  3 siblings, 1 reply; 10+ messages in thread
From: Chris Metcalf @ 2016-04-05 17:38 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, Andy Lutomirski,
	linux-doc, linux-api, linux-kernel
  Cc: Chris Metcalf

With task_isolation mode, the task is in principle guaranteed not to
be interrupted by the kernel, but only if it behaves.  In particular,
if it enters the kernel via system call, page fault, or any of a
number of other synchronous traps, it may be unexpectedly exposed
to long latencies.  Add a simple flag that puts the process into
a state where any such kernel entry generates a signal.  For system
calls, this test is performed immediately before the SECCOMP test
and causes the syscall to return immediately with ENOSYS.

By default, the task is signalled with SIGKILL, but we add prctl()
bits to support requesting a specific signal instead.

To allow the state to be entered and exited, the syscall checking
test ignores the prctl() syscall so that we can clear the bit again
later, and ignores exit/exit_group to allow exiting the task without
a pointless signal killing you as you try to do so.

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
---
 include/linux/isolation.h  | 10 +++++++
 include/uapi/linux/prctl.h |  3 ++
 kernel/isolation.c         | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/include/linux/isolation.h b/include/linux/isolation.h
index 99b909462e64..eb78175ed811 100644
--- a/include/linux/isolation.h
+++ b/include/linux/isolation.h
@@ -36,6 +36,14 @@ static inline void task_isolation_set_flags(struct task_struct *p,
 		clear_tsk_thread_flag(p, TIF_TASK_ISOLATION);
 }
 
+extern int task_isolation_syscall(int nr);
+extern void _task_isolation_exception(const char *fmt, ...);
+#define task_isolation_exception(fmt, ...)				\
+	do {								\
+		if (current_thread_info()->flags & _TIF_TASK_ISOLATION) \
+			_task_isolation_exception(fmt, ## __VA_ARGS__); \
+	} while (0)
+
 #else
 static inline void task_isolation_init(void) { }
 static inline bool task_isolation_possible(int cpu) { return false; }
@@ -43,6 +51,8 @@ static inline bool task_isolation_ready(void) { return true; }
 static inline void task_isolation_enter(void) { }
 extern inline void task_isolation_set_flags(struct task_struct *p,
 					    unsigned int flags) { }
+static inline int task_isolation_syscall(int nr) { return 0; }
+static inline void task_isolation_exception(const char *fmt, ...) { }
 #endif
 
 #endif
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 67224df4b559..a5582ace987f 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -201,5 +201,8 @@ struct prctl_mm_map {
 #define PR_SET_TASK_ISOLATION		48
 #define PR_GET_TASK_ISOLATION		49
 # define PR_TASK_ISOLATION_ENABLE	(1 << 0)
+# define PR_TASK_ISOLATION_STRICT	(1 << 1)
+# define PR_TASK_ISOLATION_SET_SIG(sig)	(((sig) & 0x7f) << 8)
+# define PR_TASK_ISOLATION_GET_SIG(bits) (((bits) >> 8) & 0x7f)
 
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/isolation.c b/kernel/isolation.c
index b364182dd8e2..f44e90109472 100644
--- a/kernel/isolation.c
+++ b/kernel/isolation.c
@@ -11,6 +11,8 @@
 #include <linux/vmstat.h>
 #include <linux/isolation.h>
 #include <linux/syscalls.h>
+#include <asm/unistd.h>
+#include <asm/syscall.h>
 #include "time/tick-sched.h"
 
 cpumask_var_t task_isolation_map;
@@ -157,3 +159,74 @@ void task_isolation_enter(void)
 	if (!tick_nohz_tick_stopped())
 		set_tsk_need_resched(current);
 }
+
+void task_isolation_interrupt(struct task_struct *task, const char *buf)
+{
+	siginfo_t info = {};
+	int sig;
+
+	pr_warn("%s/%d: task_isolation strict mode violated by %s\n",
+		task->comm, task->pid, buf);
+
+	/* Get the signal number to use. */
+	sig = PR_TASK_ISOLATION_GET_SIG(task->task_isolation_flags);
+	if (sig == 0)
+		sig = SIGKILL;
+	info.si_signo = sig;
+
+	/*
+	 * Turn off task isolation mode entirely to avoid spamming
+	 * the process with signals.  It can re-enable task isolation
+	 * mode in the signal handler if it wants to.
+	 */
+	task_isolation_set_flags(task, 0);
+
+	send_sig_info(sig, &info, task);
+}
+
+/*
+ * This routine is called from any userspace exception that doesn't
+ * otherwise trigger a signal to the user process (e.g. simple page fault).
+ */
+void _task_isolation_exception(const char *fmt, ...)
+{
+	struct task_struct *task = current;
+
+	/* RCU should have been enabled prior to this point. */
+	RCU_LOCKDEP_WARN(!rcu_is_watching(), "kernel entry without RCU");
+
+	if (task->task_isolation_flags & PR_TASK_ISOLATION_STRICT) {
+		va_list args;
+		char buf[100];
+
+		va_start(args, fmt);
+		vsnprintf(buf, sizeof(buf), fmt, args);
+		va_end(args);
+
+		task_isolation_interrupt(task, buf);
+	}
+}
+
+/*
+ * This routine is called from syscall entry (with the syscall number
+ * passed in), and in STRICT mode prevents most syscalls from executing
+ * and raises a signal to notify the process.
+ */
+int task_isolation_syscall(int syscall)
+{
+	struct task_struct *task = current;
+
+	if ((task->task_isolation_flags & PR_TASK_ISOLATION_STRICT) &&
+	    syscall != __NR_prctl &&
+	    syscall != __NR_exit && syscall != __NR_exit_group) {
+		char buf[20];
+
+		snprintf(buf, sizeof(buf), "syscall %d", syscall);
+		task_isolation_interrupt(task, buf);
+
+		syscall_set_return_value(task, current_pt_regs(), ENOSYS, -1);
+		return -1;
+	}
+
+	return 0;
+}
-- 
2.7.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 08/13] task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag
  2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
  2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
  2016-04-05 17:38 ` [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
@ 2016-04-05 17:38 ` Chris Metcalf
  2016-05-12 18:26 ` [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Metcalf @ 2016-04-05 17:38 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, Andy Lutomirski,
	linux-doc, linux-api, linux-kernel
  Cc: Chris Metcalf

When this flag is set by the initial prctl(), the semantics of task
isolation change to be "one-shot", i.e. as soon as the kernel is
re-entered for any reason, task isolation is turned off.

During application development, use of this flag is best coupled with
STRICT mode, since otherwise any bug (e.g. an munmap from another
thread in the same task causing an IPI TLB flush) could cause the
task to fall out of task isolation mode without being aware of it.

In production it is typically still best to use STRICT mode, with
a signal handler that will report violations of task isolation
up to the application layer.  However, if you are confident the
application will never fall out of task isolation mode, you may
wish to use ONE_SHOT mode to allow switching from userspace task
isolation mode, to using the kernel freely, without the small extra
penalty of invoking prctl() explicitly to turn task isolation off
before starting to use kernel services.

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
---
 include/uapi/linux/prctl.h | 1 +
 kernel/isolation.c         | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index a5582ace987f..1e204f1a0f4a 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -202,6 +202,7 @@ struct prctl_mm_map {
 #define PR_GET_TASK_ISOLATION		49
 # define PR_TASK_ISOLATION_ENABLE	(1 << 0)
 # define PR_TASK_ISOLATION_STRICT	(1 << 1)
+# define PR_TASK_ISOLATION_ONE_SHOT	(1 << 2)
 # define PR_TASK_ISOLATION_SET_SIG(sig)	(((sig) & 0x7f) << 8)
 # define PR_TASK_ISOLATION_GET_SIG(bits) (((bits) >> 8) & 0x7f)
 
diff --git a/kernel/isolation.c b/kernel/isolation.c
index 1c4f320a24a0..d0e94505bfac 100644
--- a/kernel/isolation.c
+++ b/kernel/isolation.c
@@ -205,7 +205,11 @@ void _task_isolation_exception(const char *fmt, ...)
 		va_end(args);
 
 		task_isolation_interrupt(task, buf);
+		return;
 	}
+
+	if (task->task_isolation_flags & PR_TASK_ISOLATION_ONE_SHOT)
+		task_isolation_set_flags(task, 0);
 }
 
 /*
@@ -229,6 +233,9 @@ int task_isolation_syscall(int syscall)
 		return -1;
 	}
 
+	if (task->task_isolation_flags & PR_TASK_ISOLATION_ONE_SHOT)
+		task_isolation_set_flags(task, 0);
+
 	return 0;
 }
 
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 00/13] support "task_isolation" mode
  2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
                   ` (2 preceding siblings ...)
  2016-04-05 17:38 ` [PATCH v12 08/13] task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag Chris Metcalf
@ 2016-05-12 18:26 ` Chris Metcalf
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Metcalf @ 2016-05-12 18:26 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, Andy Lutomirski,
	Daniel Lezcano, linux-doc, linux-api, linux-kernel

Ping, since the 4.7 merge window is opening soon and I haven't received
too much feedback on this version of the patch series based on 4.6-rc1.

1. Patch 09/13 for timer ticks was acked by Daniel Lezcano and is standalone,
    so could be taken into the relevant trees.  I'm not sure if it should go in
    as two separate patches through the tile and arm architecture trees,
    or through the timer tree as a combined patch.  Catalin/Will, any ideas?

    http://lkml.kernel.org/g/1459877922-15512-10-git-send-email-cmetcalf@mellanox.com

2. Patch 12/13, factoring the work_pending state machine for ARM64 into C,
    should go via the arm64 tree.  Mark Rutland should probably Ack it but then
    it should go via the ARM64 tree:

    http://lkml.kernel.org/g/1459877922-15512-13-git-send-email-cmetcalf@mellanox.com

3. Frederick provided some more feedback and I think we are still waiting
    to close the loop on the notion of how strict we should be by default:

    http://lkml.kernel.org/g/571E7FC9.60405@mellanox.com

We have been flogging this patch series along for just over a year now;
v1 of the patch series was sent on May 8, 2015.  Phew!

On 4/5/2016 1:38 PM, Chris Metcalf wrote:
> Here is a respin of the task-isolation patch set.  The previous one
> came out just before the merge window for 4.6 opened, so I suspect
> folks may have been busy merging, since it got few comments.
>
> Frederic, how are you feeling about taking this all via your tree?
> And what is your take on the new PR_TASK_ISOLATION_ONE_SHOT mode?
> I'm not sure what the right path to upstream for this series is.
>
> Changes since v11:
>
> - Rebased on v4.6-rc1.  This required me to create a
>    can_stop_my_full_tick() helper in tick-sched.c, since the underlying
>    can_stop_full_tick() now takes a struct tick_sched.
>
> - Added a HAVE_ARCH_TASK_ISOLATION Kconfig flag so that you can't
>    try to build with TASK_ISOLATION enabled for an architecture until
>    it is explicitly configured to work.  This avoids possible
>    allyesconfig build failures for unsupported architectures, or even
>    for supported ones when bisecting to the middle of this series.
>
> - Return EAGAIN instead of EINVAL for the enabling prctl() if the task
>    is affinitized to a task-isolation core, but things just aren't yet
>    right for it (e.g. another task running).  This lets the caller
>    differentiate a potentially transient failure from a permanent
>    failure, for which we still return EINVAL.
>
> The previous (v11) patch series is here:
>
> https://lkml.kernel.org/r/1457734223-26209-1-git-send-email-cmetcalf@mellanox.com
>
> This version of the patch series has been tested on arm64 and tile,
> and build-tested on x86.
>
> It remains true that the 1 Hz tick needs to be disabled for this
> patch series to be able to achieve its primary goal of enabling
> truly tick-free operation, but that is ongoing orthogonal work.
>
> The series is available at:
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git dataplane
>
> Chris Metcalf (13):
>    vmstat: add quiet_vmstat_sync function
>    vmstat: add vmstat_idle function
>    lru_add_drain_all: factor out lru_add_drain_needed
>    task_isolation: add initial support
>    task_isolation: support CONFIG_TASK_ISOLATION_ALL
>    task_isolation: support PR_TASK_ISOLATION_STRICT mode
>    task_isolation: add debug boot flag
>    task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag
>    arm, tile: turn off timer tick for oneshot_stopped state
>    arch/x86: enable task isolation functionality
>    arch/tile: enable task isolation functionality
>    arm64: factor work_pending state machine to C
>    arch/arm64: enable task isolation functionality
>
>   Documentation/kernel-parameters.txt    |  16 ++
>   arch/arm64/Kconfig                     |   1 +
>   arch/arm64/include/asm/thread_info.h   |   5 +-
>   arch/arm64/kernel/entry.S              |  12 +-
>   arch/arm64/kernel/ptrace.c             |  15 +-
>   arch/arm64/kernel/signal.c             |  42 ++++-
>   arch/arm64/kernel/smp.c                |   2 +
>   arch/arm64/mm/fault.c                  |   4 +
>   arch/tile/Kconfig                      |   1 +
>   arch/tile/include/asm/thread_info.h    |   4 +-
>   arch/tile/kernel/process.c             |   9 +
>   arch/tile/kernel/ptrace.c              |   7 +
>   arch/tile/kernel/single_step.c         |   5 +
>   arch/tile/kernel/smp.c                 |  28 +--
>   arch/tile/kernel/time.c                |   1 +
>   arch/tile/kernel/unaligned.c           |   3 +
>   arch/tile/mm/fault.c                   |   3 +
>   arch/tile/mm/homecache.c               |   2 +
>   arch/x86/Kconfig                       |   1 +
>   arch/x86/entry/common.c                |  18 +-
>   arch/x86/include/asm/thread_info.h     |   2 +
>   arch/x86/kernel/traps.c                |   2 +
>   arch/x86/mm/fault.c                    |   2 +
>   drivers/base/cpu.c                     |  18 ++
>   drivers/clocksource/arm_arch_timer.c   |   2 +
>   include/linux/context_tracking_state.h |   6 +
>   include/linux/isolation.h              |  63 +++++++
>   include/linux/sched.h                  |   3 +
>   include/linux/swap.h                   |   1 +
>   include/linux/tick.h                   |   2 +
>   include/linux/vmstat.h                 |   4 +
>   include/uapi/linux/prctl.h             |   9 +
>   init/Kconfig                           |  33 ++++
>   kernel/Makefile                        |   1 +
>   kernel/fork.c                          |   3 +
>   kernel/irq_work.c                      |   5 +-
>   kernel/isolation.c                     | 316 +++++++++++++++++++++++++++++++++
>   kernel/sched/core.c                    |  18 ++
>   kernel/signal.c                        |   8 +
>   kernel/smp.c                           |   6 +-
>   kernel/softirq.c                       |  33 ++++
>   kernel/sys.c                           |   9 +
>   kernel/time/tick-sched.c               |  36 ++--
>   mm/swap.c                              |  15 +-
>   mm/vmstat.c                            |  21 +++
>   45 files changed, 743 insertions(+), 54 deletions(-)
>   create mode 100644 include/linux/isolation.h
>   create mode 100644 kernel/isolation.c
>

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 04/13] task_isolation: add initial support
  2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
@ 2016-05-18 13:34   ` Peter Zijlstra
  2016-05-18 16:34     ` Chris Metcalf
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2016-05-18 13:34 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Andrew Morton,
	Rik van Riel, Tejun Heo, Frederic Weisbecker, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	Catalin Marinas, Will Deacon, Andy Lutomirski, Michal Hocko,
	linux-mm, linux-doc, linux-api, linux-kernel

On Tue, Apr 05, 2016 at 01:38:33PM -0400, Chris Metcalf wrote:
> diff --git a/kernel/signal.c b/kernel/signal.c
> index aa9bf00749c1..53e4e62f2778 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -34,6 +34,7 @@
>  #include <linux/compat.h>
>  #include <linux/cn_proc.h>
>  #include <linux/compiler.h>
> +#include <linux/isolation.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/signal.h>
> @@ -2213,6 +2214,9 @@ relock:
>  		/* Trace actually delivered signals. */
>  		trace_signal_deliver(signr, &ksig->info, ka);
>  
> +		/* Disable task isolation when delivering a signal. */

Why !? Changelog is quiet on this.

> +		task_isolation_set_flags(current, 0);
> +
>  		if (ka->sa.sa_handler == SIG_IGN) /* Do nothing.  */
>  			continue;
>  		if (ka->sa.sa_handler != SIG_DFL) {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode
       [not found]   ` <1459877922-15512-7-git-send-email-cmetcalf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2016-05-18 13:44     ` Peter Zijlstra
  2016-05-18 16:34       ` Chris Metcalf
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2016-05-18 13:44 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Andrew Morton,
	Rik van Riel, Tejun Heo, Frederic Weisbecker, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	Catalin Marinas, Will Deacon, Andy Lutomirski,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Tue, Apr 05, 2016 at 01:38:35PM -0400, Chris Metcalf wrote:
> +void task_isolation_interrupt(struct task_struct *task, const char *buf)
> +{
> +	siginfo_t info = {};
> +	int sig;
> +
> +	pr_warn("%s/%d: task_isolation strict mode violated by %s\n",
> +		task->comm, task->pid, buf);
> +

So the function name suggests this is called for interrupts, except its
purpose is to deliver a signal.

Now, in case of exceptions the violation isn't necessarily _by_ the task
itself. You might want to change that to report the exception
type/number instead of the affected task.

> +	/* Get the signal number to use. */
> +	sig = PR_TASK_ISOLATION_GET_SIG(task->task_isolation_flags);
> +	if (sig == 0)
> +		sig = SIGKILL;
> +	info.si_signo = sig;
> +
> +	/*
> +	 * Turn off task isolation mode entirely to avoid spamming
> +	 * the process with signals.  It can re-enable task isolation
> +	 * mode in the signal handler if it wants to.
> +	 */
> +	task_isolation_set_flags(task, 0);
> +
> +	send_sig_info(sig, &info, task);
> +}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 04/13] task_isolation: add initial support
  2016-05-18 13:34   ` Peter Zijlstra
@ 2016-05-18 16:34     ` Chris Metcalf
  2016-05-18 17:09       ` Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Metcalf @ 2016-05-18 16:34 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Andrew Morton,
	Rik van Riel, Tejun Heo, Frederic Weisbecker, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	Catalin Marinas, Will Deacon, Andy Lutomirski, Michal Hocko,
	linux-mm, linux-doc, linux-api, linux-kernel

On 5/18/2016 9:34 AM, Peter Zijlstra wrote:
> On Tue, Apr 05, 2016 at 01:38:33PM -0400, Chris Metcalf wrote:
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index aa9bf00749c1..53e4e62f2778 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -34,6 +34,7 @@
>>   #include <linux/compat.h>
>>   #include <linux/cn_proc.h>
>>   #include <linux/compiler.h>
>> +#include <linux/isolation.h>
>>   
>>   #define CREATE_TRACE_POINTS
>>   #include <trace/events/signal.h>
>> @@ -2213,6 +2214,9 @@ relock:
>>   		/* Trace actually delivered signals. */
>>   		trace_signal_deliver(signr, &ksig->info, ka);
>>   
>> +		/* Disable task isolation when delivering a signal. */
> Why !? Changelog is quiet on this.

There are really two reasons.

1. If the task is receiving a signal, it will know it's not isolated
    any more, so we don't need to worry about notifying it explicitly.
    This behavior is easy to document and allows the application to decide
    if the signal is unexpected and it should go straight to its error
    handling path (likely outcome, and in that case you want task isolation
    off anyway) or if it thinks it can plausibly re-enable isolation and
    return to where the signal interrupted you at (hard to imagine how this
    would ever make sense, but you could if you wanted to).

2. When we are delivering a signal we may already be holding the lock
    for the signal subsystem, and it gets hard to figure out whether it's
    safe to send another signal to the application as a "task isolation
    broken" notification.  For example, sending a signal to a task on
    another core involves doing an IPI to that core to kick it; the IPI
    normally is a generic point for notifying the remote core of broken
    task isolation and sending a signal - except that at the point where
    we would do that on the signal path we are already holding the lock,
    so we end up deadlocked.  We could no doubt work around that, but it
    seemed cleaner to decouple the existing signal mechanism from the
    signal delivery for task isolation.

I will add more discussion of the rationale to the commit message.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode
  2016-05-18 13:44     ` Peter Zijlstra
@ 2016-05-18 16:34       ` Chris Metcalf
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Metcalf @ 2016-05-18 16:34 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Andrew Morton,
	Rik van Riel, Tejun Heo, Frederic Weisbecker, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	Catalin Marinas, Will Deacon, Andy Lutomirski, linux-doc,
	linux-api, linux-kernel

On 5/18/2016 9:44 AM, Peter Zijlstra wrote:
> On Tue, Apr 05, 2016 at 01:38:35PM -0400, Chris Metcalf wrote:
>> +void task_isolation_interrupt(struct task_struct *task, const char *buf)
>> +{
>> +	siginfo_t info = {};
>> +	int sig;
>> +
>> +	pr_warn("%s/%d: task_isolation strict mode violated by %s\n",
>> +		task->comm, task->pid, buf);
>> +
> So the function name suggests this is called for interrupts, except its
> purpose is to deliver a signal.

Fair point.  I'll name it task_isolation_deliver_signal() in the next patch series.

> Now, in case of exceptions the violation isn't necessarily _by_ the task
> itself. You might want to change that to report the exception
> type/number instead of the affected task.

Well, we do report whatever exception information we have.  For example
a page fault exception will report the address or whatever other info is
handy; it's easy to tune since it's just a vsnprintf of some varargs from the
architecture layer.

For things like IPIs or TLB invalidations or whatever, the code currently just
reports "interrupt"; I could arrange to pass down more informative varargs
from the caller for that as well.  Let me look into it.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 04/13] task_isolation: add initial support
  2016-05-18 16:34     ` Chris Metcalf
@ 2016-05-18 17:09       ` Peter Zijlstra
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2016-05-18 17:09 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Andrew Morton,
	Rik van Riel, Tejun Heo, Frederic Weisbecker, Thomas Gleixner,
	Paul E. McKenney, Christoph Lameter, Viresh Kumar,
	Catalin Marinas, Will Deacon, Andy Lutomirski, Michal Hocko,
	linux-mm, linux-doc, linux-api, linux-kernel

On Wed, May 18, 2016 at 12:34:22PM -0400, Chris Metcalf wrote:
> On 5/18/2016 9:34 AM, Peter Zijlstra wrote:
> >On Tue, Apr 05, 2016 at 01:38:33PM -0400, Chris Metcalf wrote:
> >>diff --git a/kernel/signal.c b/kernel/signal.c
> >>index aa9bf00749c1..53e4e62f2778 100644
> >>--- a/kernel/signal.c
> >>+++ b/kernel/signal.c
> >>@@ -34,6 +34,7 @@
> >>  #include <linux/compat.h>
> >>  #include <linux/cn_proc.h>
> >>  #include <linux/compiler.h>
> >>+#include <linux/isolation.h>
> >>  #define CREATE_TRACE_POINTS
> >>  #include <trace/events/signal.h>
> >>@@ -2213,6 +2214,9 @@ relock:
> >>  		/* Trace actually delivered signals. */
> >>  		trace_signal_deliver(signr, &ksig->info, ka);
> >>+		/* Disable task isolation when delivering a signal. */
> >Why !? Changelog is quiet on this.
> 
> There are really two reasons.
> 
> 1. If the task is receiving a signal, it will know it's not isolated
>    any more, so we don't need to worry about notifying it explicitly.
>    This behavior is easy to document and allows the application to decide
>    if the signal is unexpected and it should go straight to its error
>    handling path (likely outcome, and in that case you want task isolation
>    off anyway) or if it thinks it can plausibly re-enable isolation and
>    return to where the signal interrupted you at (hard to imagine how this
>    would ever make sense, but you could if you wanted to).
> 
> 2. When we are delivering a signal we may already be holding the lock
>    for the signal subsystem, and it gets hard to figure out whether it's
>    safe to send another signal to the application as a "task isolation
>    broken" notification.  For example, sending a signal to a task on
>    another core involves doing an IPI to that core to kick it; the IPI
>    normally is a generic point for notifying the remote core of broken
>    task isolation and sending a signal - except that at the point where
>    we would do that on the signal path we are already holding the lock,
>    so we end up deadlocked.  We could no doubt work around that, but it
>    seemed cleaner to decouple the existing signal mechanism from the
>    signal delivery for task isolation.
> 
> I will add more discussion of the rationale to the commit message.

Please also expand the in-code comment, as that is what we'll see first
when reading the code.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-05-18 17:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
2016-05-18 13:34   ` Peter Zijlstra
2016-05-18 16:34     ` Chris Metcalf
2016-05-18 17:09       ` Peter Zijlstra
2016-04-05 17:38 ` [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
     [not found]   ` <1459877922-15512-7-git-send-email-cmetcalf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2016-05-18 13:44     ` Peter Zijlstra
2016-05-18 16:34       ` Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 08/13] task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag Chris Metcalf
2016-05-12 18:26 ` [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).