[PATCH 0/9] powerpc checkpoint/restart kernel support

[PATCH 0/9] powerpc checkpoint/restart kernel support

[Nathan Lynch]

Patches 1-3 fix general issues and can be applied immediately.

Patches 4-8 implement the checkpoint and restart syscalls and
architecture callbacks.

Patch 9 implements the clone_with_pids syscall, but I think the shape
of that interface is yet to be finalized, so I guess it's FYI for now.

Simple single task checkpoint and restart work, as does
clone_with_pids, apparently.  Multiple task restart fails; the
coordinator task appears to be receiving a signal during
wait_all_tasks_finish() -> wait_for_completion_interruptible() for
some reason I am still trying to tracking down.

Nathan Lynch (9):
  checkpoint: fix integer-pointer conversion warnings
  fix trivial build breaks in compat code
  futex.h: fix compat builds
  powerpc: reserve checkpoint arch identifiers
  powerpc: provide APIs for validating and updating DABR
  powerpc: checkpoint/restart implementation
  powerpc: wire up checkpoint and restart syscalls
  powerpc: enable checkpoint support in Kconfig
  powerpc: clone_with_pids implementation

 arch/powerpc/Kconfig                      |    3 +
 arch/powerpc/include/asm/Kbuild           |    1 +
 arch/powerpc/include/asm/checkpoint_hdr.h |   22 ++
 arch/powerpc/include/asm/ptrace.h         |    7 +
 arch/powerpc/include/asm/syscalls.h       |    4 +
 arch/powerpc/include/asm/systbl.h         |    3 +
 arch/powerpc/include/asm/unistd.h         |    5 +-
 arch/powerpc/kernel/entry_32.S            |    8 +
 arch/powerpc/kernel/entry_64.S            |    5 +
 arch/powerpc/kernel/process.c             |   18 +
 arch/powerpc/kernel/ptrace.c              |   88 ++++--
 arch/powerpc/mm/Makefile                  |    1 +
 arch/powerpc/mm/checkpoint.c              |  531 +++++++++++++++++++++++++++++
 checkpoint/process.c                      |   14 +-
 include/linux/checkpoint_hdr.h            |    2 +
 include/linux/futex.h                     |    4 +
 16 files changed, 679 insertions(+), 37 deletions(-)
 create mode 100644 arch/powerpc/include/asm/checkpoint_hdr.h
 create mode 100644 arch/powerpc/mm/checkpoint.c

[PATCH 1/9] checkpoint: fix integer-pointer conversion warnings

[Nathan Lynch]

  CC      checkpoint/process.o
checkpoint/process.c: In function ‘checkpoint_task_struct’:
checkpoint/process.c:97: warning: assignment makes integer from pointer without a cast
checkpoint/process.c:98: warning: assignment makes integer from pointer without a cast
checkpoint/process.c: In function ‘restore_task_struct’:
checkpoint/process.c:475: warning: assignment makes pointer from integer without a cast
checkpoint/process.c:476: warning: assignment makes pointer from integer without a cast

Signed-off-by: Nathan Lynch <ntl@pobox.com>
---
 checkpoint/process.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/checkpoint/process.c b/checkpoint/process.c
index 41566db..b7f4a43 100644
--- a/checkpoint/process.c
+++ b/checkpoint/process.c
@@ -94,8 +94,8 @@ static int checkpoint_task_struct(struct ckpt_ctx *ctx, struct task_struct *t)
 		h->exit_signal = t->exit_signal;
 		h->pdeath_signal = t->pdeath_signal;
 
-		h->set_child_tid = t->set_child_tid;
-		h->clear_child_tid = t->clear_child_tid;
+		h->set_child_tid = (unsigned long)t->set_child_tid;
+		h->clear_child_tid = (unsigned long)t->clear_child_tid;
 		save_task_robust_futex_list(h, t);
 	}
 
@@ -472,8 +472,8 @@ static int restore_task_struct(struct ckpt_ctx *ctx)
 		t->exit_signal = h->exit_signal;
 		t->pdeath_signal = h->pdeath_signal;
 
-		t->set_child_tid = h->set_child_tid;
-		t->clear_child_tid = h->clear_child_tid;
+		t->set_child_tid = (void __user *)h->set_child_tid;
+		t->clear_child_tid = (void __user *)h->clear_child_tid;
 		restore_task_robust_futex_list(h);
 	}
 
-- 
1.6.0.6

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

[PATCH 2/9] fix trivial build breaks in compat code

[Nathan Lynch]

---
 checkpoint/process.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/checkpoint/process.c b/checkpoint/process.c
index b7f4a43..41c1c85 100644
--- a/checkpoint/process.c
+++ b/checkpoint/process.c
@@ -308,7 +308,7 @@ int checkpoint_restart_block(struct ckpt_ctx *ctx, struct task_struct *t)
 #ifdef CONFIG_COMPAT
 	} else if (fn == compat_nanosleep_restart) {
 
-		h->function_type = CKPT_RESTART_BLOCK_NANOSLEEP;
+		h->function_type = CKPT_RESTART_BLOCK_COMPAT_NANOSLEEP;
 		h->arg_0 = restart_block->nanosleep.index;
 		h->arg_1 = (unsigned long)restart_block->nanosleep.rmtp;
 		h->arg_2 = (unsigned long)restart_block->nanosleep.compat_rmtp;
@@ -670,7 +670,7 @@ int restore_restart_block(struct ckpt_ctx *ctx)
 		restart_block.nanosleep.compat_rmtp =
 			(struct compat_timespec __user *)
 				(unsigned long) h->arg_2;
-		resatrt_block.nanosleep.expires = expire;
+		restart_block.nanosleep.expires = expire;
 		break;
 	case CKPT_RESTART_BLOCK_COMPAT_CLOCK_NANOSLEEP:
 		clockid = h->arg_0;
@@ -683,7 +683,7 @@ int restore_restart_block(struct ckpt_ctx *ctx)
 		restart_block.nanosleep.compat_rmtp =
 			(struct compat_timespec __user *)
 				(unsigned long) h->arg_2;
-		resatrt_block.nanosleep.expires = expire;
+		restart_block.nanosleep.expires = expire;
 		break;
 #endif
 	case CKPT_RESTART_BLOCK_FUTEX:
-- 
1.6.0.6

[PATCH 3/9] futex.h: fix compat builds

[Nathan Lynch]

With any build where CONFIG_COMPAT=y:

checkpoint/process.c: In function 'save_task_robust_futex_list':
checkpoint/process.c:37: error: implicit declaration of function 'ptr_to_compat'
checkpoint/process.c:38: error: dereferencing pointer to incomplete type
checkpoint/process.c: In function 'restore_task_robust_futex_list':
checkpoint/process.c:54: error: implicit declaration of function 'compat_ptr'
checkpoint/process.c:54: warning: assignment makes pointer from integer without a cast
checkpoint/process.c:55: error: implicit declaration of function 'do_compat_set_robust_list'

ptr_to_compat et al need compat.h; however, futex.h is exported while
compat.h is not.  So the include is guarded by #ifdef __KERNEL__.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 include/linux/futex.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index 2e126a9..3fb9f33 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -4,6 +4,10 @@
 #include <linux/compiler.h>
 #include <linux/types.h>
 
+#ifdef __KERNEL__
+#include <linux/compat.h>
+#endif
+
 struct inode;
 struct mm_struct;
 struct task_struct;
-- 
1.6.0.6

[PATCH 4/9] powerpc: reserve checkpoint arch identifiers

[Nathan Lynch]

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 include/linux/checkpoint_hdr.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 06bc6e2..851cdf2 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -110,6 +110,8 @@ enum {
 	/* do not change order (will break ABI) */
 	CKPT_ARCH_X86_32 = 1,
 	CKPT_ARCH_S390X,
+	CKPT_ARCH_PPC32,
+	CKPT_ARCH_PPC64,
 };
 
 /* shared objrects (objref) */
-- 
1.6.0.6

[PATCH 5/9] powerpc: provide APIs for validating and updating DABR

[Nathan Lynch]

A checkpointed task image may specify a value for the DABR (Data
Access Breakpoint Register).  The restart code needs to validate this
value before making any changes to the current task.

ptrace_set_debugreg encapsulates the bounds checking and platform
dependencies of programming the DABR.  Split this into "validate"
(debugreg_valid) and "update" (debugreg_update) functions, and make
them available for use outside of the ptrace code.

Also ptrace_set_debugreg has extern linkage, but no users outside of
ptrace.c.  Make it static.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/include/asm/ptrace.h |    7 +++
 arch/powerpc/kernel/ptrace.c      |   88 +++++++++++++++++++++++++------------
 2 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 8c34149..c6cb2c6 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -81,6 +81,8 @@ struct pt_regs {
 
 #ifndef __ASSEMBLY__
 
+#include <linux/types.h>
+
 #define instruction_pointer(regs) ((regs)->nip)
 #define user_stack_pointer(regs) ((regs)->gpr[1])
 #define regs_return_value(regs) ((regs)->gpr[3])
@@ -140,6 +142,11 @@ extern void user_enable_single_step(struct task_struct *);
 extern void user_enable_block_step(struct task_struct *);
 extern void user_disable_single_step(struct task_struct *);
 
+/* for reprogramming DABR/DAC during restart of a checkpointed task */
+extern bool debugreg_valid(unsigned long val, unsigned int index);
+extern void debugreg_update(struct task_struct *task, unsigned long val,
+			    unsigned int index);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 9fa2c7d..15ab4be 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -754,22 +754,25 @@ void user_disable_single_step(struct task_struct *task)
 	clear_tsk_thread_flag(task, TIF_SINGLESTEP);
 }
 
-int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
-			       unsigned long data)
+/**
+ * debugreg_valid() - validate the value to be written to a debug register
+ * @val:	The prospective contents of the register.
+ * @index:	Must be zero.
+ *
+ * Returns true if @val is an acceptable value for the register indicated by
+ * @index, false otherwise.
+ */
+bool debugreg_valid(unsigned long val, unsigned int index)
 {
-	/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
-	 *  For embedded processors we support one DAC and no IAC's at the
-	 *  moment.
-	 */
-	if (addr > 0)
-		return -EINVAL;
+	/* We support only one debug register for now */
+	if (index != 0)
+		return false;
 
 	/* The bottom 3 bits in dabr are flags */
-	if ((data & ~0x7UL) >= TASK_SIZE)
-		return -EIO;
+	if ((val & ~0x7UL) >= TASK_SIZE)
+		return false;
 
 #ifndef CONFIG_BOOKE
-
 	/* For processors using DABR (i.e. 970), the bottom 3 bits are flags.
 	 *  It was assumed, on previous implementations, that 3 bits were
 	 *  passed together with the data address, fitting the design of the
@@ -783,47 +786,74 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 	 */
 
 	/* Ensure breakpoint translation bit is set */
-	if (data && !(data & DABR_TRANSLATION))
-		return -EIO;
-
-	/* Move contents to the DABR register */
-	task->thread.dabr = data;
-
-#endif
-#if defined(CONFIG_BOOKE)
-
+	if (val && !(val & DABR_TRANSLATION))
+		return false;
+#else
 	/* As described above, it was assumed 3 bits were passed with the data
 	 *  address, but we will assume only the mode bits will be passed
 	 *  as to not cause alignment restrictions for DAC-based processors.
 	 */
 
+	/* Read or Write bits must be set */
+	if (!(val & 0x3UL))
+		return -EINVAL;
+#endif
+	return true;
+}
+
+/**
+ * debugreg_update() - update a debug register associated with a task
+ * @task:	The task whose register state is to be modified.
+ * @val:	The value to be written to the debug register.
+ * @index:	Specifies the debug register.  Currently unused.
+ *
+ * Set a task's DABR/DAC to @val, which should be validated with
+ * debugreg_valid() beforehand.
+ */
+void debugreg_update(struct task_struct *task, unsigned long val,
+		     unsigned int index)
+{
+#ifndef CONFIG_BOOKE
+	task->thread.dabr = val;
+#else
 	/* DAC's hold the whole address without any mode flags */
-	task->thread.dabr = data & ~0x3UL;
+	task->thread.dabr = val & ~0x3UL;
 
 	if (task->thread.dabr == 0) {
 		task->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
 		task->thread.regs->msr &= ~MSR_DE;
-		return 0;
 	}
 
-	/* Read or Write bits must be set */
-
-	if (!(data & 0x3UL))
-		return -EINVAL;
-
 	/* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
 	   register */
 	task->thread.dbcr0 = DBCR0_IDM;
 
 	/* Check for write and read flags and set DBCR0
 	   accordingly */
-	if (data & 0x1UL)
+	if (val & 0x1UL)
 		task->thread.dbcr0 |= DBSR_DAC1R;
-	if (data & 0x2UL)
+	if (val & 0x2UL)
 		task->thread.dbcr0 |= DBSR_DAC1W;
 
 	task->thread.regs->msr |= MSR_DE;
 #endif
+}
+
+static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
+			       unsigned long data)
+{
+	/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
+	 * For embedded processors we support one DAC and no IAC's at the
+	 * moment.
+	 */
+	if (addr > 0)
+		return -EINVAL;
+
+	if (!debugreg_valid(data, 0))
+		return -EIO;
+
+	debugreg_update(task, data, 0);
+
 	return 0;
 }
 
-- 
1.6.0.6

[PATCH 6/9] powerpc: checkpoint/restart implementation

[Nathan Lynch]

Support for checkpointing and restarting GPRs, FPU state, DABR, and
Altivec state.

The portion of the checkpoint image manipulated by this code begins
with a bitmask of features indicating the various contexts saved.
Fields in image that can vary depending on kernel configuration
(e.g. FP regs due to VSX) have their sizes explicitly recorded, except
for GPRS, so migrating between ppc32 and ppc64 won't work yet.

The restart code ensures that the task is not modified until the
checkpoint image is validated against the current kernel configuration
and hardware features (e.g. can't restart a task using Altivec on
non-Altivec systems).

What works:
* self and external checkpoint of simple (single thread, one open
  file) 32- and 64-bit processes on a ppc64 kernel

What doesn't work:
* restarting a 32-bit task from a 64-bit task and vice versa

Untested:
* ppc32 (but it builds)

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/include/asm/Kbuild           |    1 +
 arch/powerpc/include/asm/checkpoint_hdr.h |   22 ++
 arch/powerpc/mm/Makefile                  |    1 +
 arch/powerpc/mm/checkpoint.c              |  531 +++++++++++++++++++++++++++++
 4 files changed, 555 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/include/asm/checkpoint_hdr.h
 create mode 100644 arch/powerpc/mm/checkpoint.c

diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 5ab7d7f..20379f1 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -12,6 +12,7 @@ header-y += shmbuf.h
 header-y += socket.h
 header-y += termbits.h
 header-y += fcntl.h
+header-y += checkpoint_hdr.h
 header-y += poll.h
 header-y += sockios.h
 header-y += ucontext.h
diff --git a/arch/powerpc/include/asm/checkpoint_hdr.h b/arch/powerpc/include/asm/checkpoint_hdr.h
new file mode 100644
index 0000000..db109b2
--- /dev/null
+++ b/arch/powerpc/include/asm/checkpoint_hdr.h
@@ -0,0 +1,22 @@
+#ifndef __ASM_POWERPC_CKPT_HDR_H
+#define __ASM_POWERPC_CKPT_HDR_H
+
+#include <linux/types.h>
+
+/* This must match _NSIG in <asm/signal.h> */
+#define CKPT_ARCH_NSIG 64
+
+#ifdef __KERNEL__
+#ifdef CONFIG_PPC64
+#define CKPT_ARCH_ID CKPT_ARCH_PPC64
+#else
+#define CKPT_ARCH_ID CKPT_ARCH_PPC32
+#endif
+#endif
+
+struct ckpt_hdr_header_arch {
+	struct ckpt_hdr h;
+	__u32 what;
+} __attribute__((aligned(8)));
+
+#endif /* __ASM_POWERPC_CKPT_HDR_H */
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 3e68363..aa8733c 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_PPC_SUBPAGE_PROT)	+= subpage-prot.o
 obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-noncoherent.o
 obj-$(CONFIG_HIGHMEM)		+= highmem.o
+obj-$(CONFIG_CHECKPOINT)	+= checkpoint.o
diff --git a/arch/powerpc/mm/checkpoint.c b/arch/powerpc/mm/checkpoint.c
new file mode 100644
index 0000000..de18467
--- /dev/null
+++ b/arch/powerpc/mm/checkpoint.c
@@ -0,0 +1,531 @@
+/*
+ * PowerPC architecture support for checkpoint/restart.
+ * Based on x86 implementation.
+ *
+ * Copyright (C) 2008 Oren Laadan
+ * Copyright 2009 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ */
+
+#if 0
+#define DEBUG
+#endif
+
+#include <linux/checkpoint.h>
+#include <linux/checkpoint_hdr.h>
+#include <linux/kernel.h>
+#include <asm/processor.h>
+#include <asm/ptrace.h>
+#include <asm/system.h>
+
+enum ckpt_cpu_feature {
+	CKPT_USED_FP,
+	CKPT_USED_DEBUG,
+	CKPT_USED_ALTIVEC,
+	CKPT_USED_SPE,
+	CKPT_USED_VSX,
+	CKPT_FTR_END = 31,
+};
+
+#define x(ftr) (1UL << ftr)
+
+/* features this kernel can handle for restart */
+enum {
+	CKPT_FTRS_POSSIBLE =
+#ifdef CONFIG_PPC_FPU
+	x(CKPT_USED_FP) |
+#endif
+	x(CKPT_USED_DEBUG) |
+#ifdef CONFIG_ALTIVEC
+	x(CKPT_USED_ALTIVEC) |
+#endif
+#ifdef CONFIG_SPE
+	x(CKPT_USED_SPE) |
+#endif
+#ifdef CONFIG_VSX
+	x(CKPT_USED_VSX) |
+#endif
+	0,
+};
+
+#undef x
+
+struct ckpt_hdr_cpu {
+	struct ckpt_hdr h;
+	u32 features_used;
+	u32 pt_regs_size;
+	u32 fpr_size;
+	u64 orig_gpr3;
+	struct pt_regs pt_regs;
+	/* relevant fields from thread_struct */
+	double fpr[32][TS_FPRWIDTH];
+	u32 fpscr;
+	s32 fpexc_mode;
+	u64 dabr;
+	/* Altivec/VMX state */
+	vector128 vr[32];
+	vector128 vscr;
+	u64 vrsave;
+	/* SPE state */
+	u32 evr[32];
+	u64 acc;
+	u32 spefscr;
+};
+
+/**************************************************************************
+ * Checkpoint
+ */
+
+static void ckpt_cpu_feature_set(struct ckpt_hdr_cpu *hdr,
+				 enum ckpt_cpu_feature ftr)
+{
+	hdr->features_used |= 1ULL << ftr;
+}
+
+static bool ckpt_cpu_feature_isset(const struct ckpt_hdr_cpu *hdr,
+				 enum ckpt_cpu_feature ftr)
+{
+	return hdr->features_used & (1ULL << ftr);
+}
+
+/* determine whether an image has feature bits set that this kernel
+ * does not support */
+static bool ckpt_cpu_features_unknown(const struct ckpt_hdr_cpu *hdr)
+{
+	return hdr->features_used & ~CKPT_FTRS_POSSIBLE;
+}
+
+static void checkpoint_gprs(struct ckpt_hdr_cpu *cpu_hdr,
+			    struct task_struct *task)
+{
+	struct pt_regs *pt_regs;
+
+	pr_debug("%s: saving GPRs\n", __func__);
+
+	cpu_hdr->pt_regs_size = sizeof(*pt_regs);
+	pt_regs = task_pt_regs(task);
+	cpu_hdr->pt_regs = *pt_regs;
+
+	if (task == current)
+		cpu_hdr->pt_regs.gpr[3] = 0;
+
+	cpu_hdr->orig_gpr3 = pt_regs->orig_gpr3;
+}
+
+#ifdef CONFIG_PPC_FPU
+static void checkpoint_fpu(struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task)
+{
+	/* easiest to save FP state unconditionally */
+
+	pr_debug("%s: saving FPU state\n", __func__);
+
+	if (task == current)
+		flush_fp_to_thread(task);
+
+	cpu_hdr->fpr_size = sizeof(cpu_hdr->fpr);
+	cpu_hdr->fpscr = task->thread.fpscr.val;
+	cpu_hdr->fpexc_mode = task->thread.fpexc_mode;
+
+	memcpy(cpu_hdr->fpr, task->thread.fpr, sizeof(cpu_hdr->fpr));
+
+	ckpt_cpu_feature_set(cpu_hdr, CKPT_USED_FP);
+}
+#else
+static void checkpoint_fpu(struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task)
+{
+	return;
+}
+#endif
+
+#ifdef CONFIG_ALTIVEC
+static void checkpoint_altivec(struct ckpt_hdr_cpu *cpu_hdr,
+			       struct task_struct *task)
+{
+	if (!cpu_has_feature(CPU_FTR_ALTIVEC))
+		return;
+
+	if (!task->thread.used_vr)
+		return;
+
+	pr_debug("%s: saving Altivec state\n", __func__);
+
+	if (task == current)
+		flush_altivec_to_thread(task);
+
+	cpu_hdr->vrsave = task->thread.vrsave;
+	memcpy(cpu_hdr->vr, task->thread.vr, sizeof(cpu_hdr->vr));
+	ckpt_cpu_feature_set(cpu_hdr, CKPT_USED_ALTIVEC);
+}
+#else
+static void checkpoint_altivec(struct ckpt_hdr_cpu *cpu_hdr,
+			       struct task_struct *task)
+{
+	return;
+}
+#endif
+
+#ifdef CONFIG_SPE
+static void checkpoint_spe(struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task)
+{
+	if (!cpu_has_feature(CPU_FTR_SPE))
+		return;
+
+	if (!task->thread.used_spe)
+		return;
+
+	pr_debug("%s: saving SPE state\n", __func__);
+
+	if (task == current)
+		flush_spe_to_thread(task);
+
+	cpu_hdr->acc = task->thread.acc;
+	cpu_hdr->spefscr = task->thread.spefscr;
+	memcpy(cpu_hdr->evr, task->thread.evr, sizeof(cpu_hdr->evr));
+	ckpt_cpu_feature_set(cpu_hdr, CKPT_USED_SPE);
+}
+#else
+static void checkpoint_spe(struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task)
+{
+	return;
+}
+#endif
+
+static void checkpoint_dabr(struct ckpt_hdr_cpu *cpu_hdr,
+			    const struct task_struct *task)
+{
+	if (!task->thread.dabr)
+		return;
+
+	cpu_hdr->dabr = task->thread.dabr;
+	ckpt_cpu_feature_set(cpu_hdr, CKPT_USED_DEBUG);
+}
+
+/* dump the thread_struct of a given task */
+int checkpoint_thread(struct ckpt_ctx *ctx, struct task_struct *t)
+{
+	return 0;
+}
+
+/* dump the cpu state and registers of a given task */
+int checkpoint_cpu(struct ckpt_ctx *ctx, struct task_struct *t)
+{
+	struct ckpt_hdr_cpu *cpu_hdr;
+	int rc;
+
+	rc = -ENOMEM;
+	cpu_hdr = ckpt_hdr_get_type(ctx, sizeof(*cpu_hdr), CKPT_HDR_CPU);
+	if (!cpu_hdr)
+		goto err;
+
+	checkpoint_gprs(cpu_hdr, t);
+	checkpoint_fpu(cpu_hdr, t);
+	checkpoint_dabr(cpu_hdr, t);
+	checkpoint_altivec(cpu_hdr, t);
+	checkpoint_spe(cpu_hdr, t);
+
+	rc = ckpt_write_obj(ctx, (struct ckpt_hdr *) cpu_hdr);
+err:
+	ckpt_hdr_put(ctx, cpu_hdr);
+	return rc;
+}
+
+int checkpoint_write_header_arch(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_header_arch *arch_hdr;
+	int ret;
+
+	arch_hdr = ckpt_hdr_get_type(ctx, sizeof(*arch_hdr),
+				     CKPT_HDR_HEADER_ARCH);
+	if (!arch_hdr)
+		return -ENOMEM;
+
+	arch_hdr->what = 0xdeadbeef;
+
+	ret = ckpt_write_obj(ctx, &arch_hdr->h);
+	ckpt_hdr_put(ctx, arch_hdr);
+
+	return ret;
+}
+
+/* dump the mm->context state */
+int checkpoint_mm_context(struct ckpt_ctx *ctx, struct mm_struct *mm)
+{
+	return 0;
+}
+
+/**************************************************************************
+ * Restart
+ */
+
+/* read the thread_struct into the current task */
+int restore_thread(struct ckpt_ctx *ctx)
+{
+	return 0;
+}
+
+/* Based on the MSR value from a checkpoint image, produce an MSR
+ * value that is appropriate for the restored task.  Right now we only
+ * check for MSR_SF (64-bit) for PPC64.
+ */
+static unsigned long sanitize_msr(unsigned long msr_ckpt)
+{
+#ifdef CONFIG_PPC32
+	return MSR_USER;
+#else
+	if (msr_ckpt & MSR_SF)
+		return MSR_USER64;
+	return MSR_USER32;
+#endif
+}
+
+static int restore_gprs(const struct ckpt_hdr_cpu *cpu_hdr,
+			struct task_struct *task, bool update)
+{
+	struct pt_regs *regs;
+	int rc;
+
+	rc = -EINVAL;
+	if (cpu_hdr->pt_regs_size != sizeof(*regs))
+		goto out;
+
+	rc = 0;
+	if (!update)
+		goto out;
+
+	regs = task_pt_regs(task);
+	*regs = cpu_hdr->pt_regs;
+
+	regs->orig_gpr3 = cpu_hdr->orig_gpr3;
+
+	regs->msr = sanitize_msr(regs->msr);
+out:
+	return rc;
+}
+
+#ifdef CONFIG_PPC_FPU
+static int restore_fpu(const struct ckpt_hdr_cpu *cpu_hdr,
+		       struct task_struct *task, bool update)
+{
+	int rc;
+
+	rc = -EINVAL;
+	if (cpu_hdr->fpr_size != sizeof(task->thread.fpr))
+		goto out;
+
+	rc = 0;
+	if (!update || !ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_FP))
+		goto out;
+
+	task->thread.fpscr.val = cpu_hdr->fpscr;
+	task->thread.fpexc_mode = cpu_hdr->fpexc_mode;
+
+	memcpy(task->thread.fpr, cpu_hdr->fpr, sizeof(task->thread.fpr));
+out:
+	return rc;
+}
+#else
+static int restore_fpu(const struct ckpt_hdr_cpu *cpu_hdr,
+		       struct task_struct *task, bool update)
+{
+	WARN_ON_ONCE(ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_FP));
+	return 0;
+}
+#endif
+
+static int restore_dabr(const struct ckpt_hdr_cpu *cpu_hdr,
+			struct task_struct *task, bool update)
+{
+	int rc;
+
+	rc = 0;
+	if (!ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_DEBUG))
+		goto out;
+
+	rc = -EINVAL;
+	if (!debugreg_valid(cpu_hdr->dabr, 0))
+		goto out;
+
+	rc = 0;
+	if (!update)
+		goto out;
+
+	debugreg_update(task, cpu_hdr->dabr, 0);
+out:
+	return rc;
+}
+
+#ifdef CONFIG_ALTIVEC
+static int restore_altivec(const struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task, bool update)
+{
+	int rc;
+
+	rc = 0;
+	if (!ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_ALTIVEC))
+		goto out;
+
+	rc = -EINVAL;
+	if (!cpu_has_feature(CPU_FTR_ALTIVEC))
+		goto out;
+
+	rc = 0;
+	if (!update)
+		goto out;
+
+	task->thread.vrsave = cpu_hdr->vrsave;
+	task->thread.used_vr = 1;
+
+	memcpy(task->thread.vr, cpu_hdr->vr, sizeof(cpu_hdr->vr));
+out:
+	return rc;
+}
+#else
+static int restore_altivec(const struct ckpt_hdr_cpu *cpu_hdr,
+			   struct task_struct *task, bool update)
+{
+	WARN_ON_ONCE(ckpt_cpu_feature_isset(CKPT_USED_ALTIVEC));
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_SPE
+static int restore_spe(const struct ckpt_hdr_cpu *cpu_hdr,
+		       struct task_struct *task, bool update)
+{
+	int rc;
+
+	rc = 0;
+	if (!ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_SPE))
+		goto out;
+
+	rc = -EINVAL;
+	if (!cpu_has_feature(CPU_FTR_SPE))
+		goto out;
+
+	rc = 0;
+	if (!update)
+		goto out;
+
+	task->thread.acc = cpu_hdr->acc;
+	task->thread.spefscr = cpu_hdr->spefscr;
+	task->thread.used_spe = 1;
+
+	memcpy(task->thread.evr, cpu_hdr->evr, sizeof(cpu_hdr->evr));
+out:
+	return rc;
+}
+#else
+static int restore_spe(const struct ckpt_hdr_cpu *cpu_hdr,
+		       struct task_struct *task, bool update)
+{
+	WARN_ON_ONCE(ckpt_cpu_feature_isset(cpu_hdr, CKPT_USED_SPE));
+	return 0;
+}
+#endif
+
+struct restore_func_desc {
+	int (*func)(const struct ckpt_hdr_cpu *, struct task_struct *, bool);
+	const char *info;
+};
+
+typedef int (*restore_func_t)(const struct ckpt_hdr_cpu *,
+			      struct task_struct *, bool);
+
+static const restore_func_t restore_funcs[] = {
+	restore_gprs,
+	restore_fpu,
+	restore_dabr,
+	restore_altivec,
+	restore_spe,
+};
+
+static bool bitness_match(const struct ckpt_hdr_cpu *cpu_hdr,
+			  const struct task_struct *task)
+{
+	/* 64-bit image */
+	if (cpu_hdr->pt_regs.msr & MSR_SF) {
+		if (task->thread.regs->msr & MSR_SF)
+			return true;
+		else
+			return false;
+	}
+
+	/* 32-bit image */
+	if (task->thread.regs->msr & MSR_SF)
+		return false;
+
+	return true;
+}
+
+int restore_cpu(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_cpu *cpu_hdr;
+	bool update;
+	int rc;
+	int i;
+
+	cpu_hdr = ckpt_read_obj_type(ctx, sizeof(*cpu_hdr), CKPT_HDR_CPU);
+	if (IS_ERR(cpu_hdr))
+		return PTR_ERR(cpu_hdr);
+
+	rc = -EINVAL;
+	if (ckpt_cpu_features_unknown(cpu_hdr))
+		goto err;
+
+	/* temporary: restoring a 32-bit image from a 64-bit task and
+	 * vice-versa is known not to work (probably not restoring
+	 * thread_info correctly); detect this and fail gracefully.
+	 */
+	if (!bitness_match(cpu_hdr, current))
+		goto err;
+
+	/* We want to determine whether there's anything wrong with
+	 * the checkpoint image before changing the task at all.  Run
+	 * a "check" phase (update = false) first.
+	 */
+	update = false;
+commit:
+	for (i = 0; i < ARRAY_SIZE(restore_funcs); i++) {
+		rc = restore_funcs[i](cpu_hdr, current, update);
+		if (rc == 0)
+			continue;
+		pr_debug("%s: restore_func[%i] failed\n", __func__, i);
+		WARN_ON_ONCE(update);
+		goto err;
+	}
+
+	if (!update) {
+		update = true;
+		goto commit;
+	}
+
+err:
+	ckpt_hdr_put(ctx, cpu_hdr);
+	return rc;
+}
+
+int restore_read_header_arch(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_header_arch *arch_hdr;
+
+	arch_hdr = ckpt_read_obj_type(ctx, sizeof(*arch_hdr),
+				      CKPT_HDR_HEADER_ARCH);
+	if (IS_ERR(arch_hdr))
+		return PTR_ERR(arch_hdr);
+
+	ckpt_hdr_put(ctx, arch_hdr);
+
+	return 0;
+}
+
+int restore_mm_context(struct ckpt_ctx *ctx, struct mm_struct *mm)
+{
+	return 0;
+}
-- 
1.6.0.6

[PATCH 7/9] powerpc: wire up checkpoint and restart syscalls

[Nathan Lynch]

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/include/asm/systbl.h |    2 ++
 arch/powerpc/include/asm/unistd.h |    4 +++-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 370600c..3d44cf3 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -326,3 +326,5 @@ SYSCALL_SPU(perf_counter_open)
 COMPAT_SYS_SPU(preadv)
 COMPAT_SYS_SPU(pwritev)
 COMPAT_SYS(rt_tgsigqueueinfo)
+SYSCALL(checkpoint)
+SYSCALL(restart)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index cef080b..ef41ebb 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -345,10 +345,12 @@
 #define __NR_preadv		320
 #define __NR_pwritev		321
 #define __NR_rt_tgsigqueueinfo	322
+#define __NR_checkpoint		323
+#define __NR_restart		324
 
 #ifdef __KERNEL__
 
-#define __NR_syscalls		323
+#define __NR_syscalls		325
 
 #define __NR__exit __NR_exit
 #define NR_syscalls	__NR_syscalls
-- 
1.6.0.6

[PATCH 8/9] powerpc: enable checkpoint support in Kconfig

[Nathan Lynch]

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/Kconfig |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d00131c..2ca160e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -26,6 +26,9 @@ config MMU
 	bool
 	default y
 
+config CHECKPOINT_SUPPORT
+	def_bool y
+
 config GENERIC_CMOS_UPDATE
 	def_bool y
 
-- 
1.6.0.6

[PATCH 9/9] powerpc: clone_with_pids implementation

[Nathan Lynch]

Mostly copy-paste from existing clone code.

(may want to hold off applying this until the ongoing clone_with_pids
discussion is resolved.)

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/include/asm/syscalls.h |    4 ++++
 arch/powerpc/include/asm/systbl.h   |    1 +
 arch/powerpc/include/asm/unistd.h   |    3 ++-
 arch/powerpc/kernel/entry_32.S      |    8 ++++++++
 arch/powerpc/kernel/entry_64.S      |    5 +++++
 arch/powerpc/kernel/process.c       |   18 ++++++++++++++++++
 6 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index eb8eb40..25dfe8b 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -24,6 +24,10 @@ asmlinkage int sys_execve(unsigned long a0, unsigned long a1,
 asmlinkage int sys_clone(unsigned long clone_flags, unsigned long usp,
 		int __user *parent_tidp, void __user *child_threadptr,
 		int __user *child_tidp, int p6, struct pt_regs *regs);
+asmlinkage int sys_clone_with_pids(unsigned long clone_flags,
+		unsigned long usp, int __user *parent_tidp,
+		void __user *child_threadptr, int __user *child_tidp,
+		void __user *upid_setp, struct pt_regs *regs);
 asmlinkage int sys_fork(unsigned long p1, unsigned long p2,
 		unsigned long p3, unsigned long p4, unsigned long p5,
 		unsigned long p6, struct pt_regs *regs);
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 3d44cf3..e3c0b96 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -328,3 +328,4 @@ COMPAT_SYS_SPU(pwritev)
 COMPAT_SYS(rt_tgsigqueueinfo)
 SYSCALL(checkpoint)
 SYSCALL(restart)
+PPC_SYS(clone_with_pids)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index ef41ebb..c54db8b 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -347,10 +347,11 @@
 #define __NR_rt_tgsigqueueinfo	322
 #define __NR_checkpoint		323
 #define __NR_restart		324
+#define __NR_clone_with_pids	325
 
 #ifdef __KERNEL__
 
-#define __NR_syscalls		325
+#define __NR_syscalls		326
 
 #define __NR__exit __NR_exit
 #define NR_syscalls	__NR_syscalls
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 3cadba6..081dd36 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -586,6 +586,14 @@ ppc_clone:
 	stw	r0,_TRAP(r1)		/* register set saved */
 	b	sys_clone
 
+	.globl	ppc_clone_with_pids
+ppc_clone_with_pids:
+	SAVE_NVGPRS(r1)
+	lwz	r0,_TRAP(r1)
+	rlwinm	r0,r0,0,0,30		/* clear LSB to indicate full */
+	stw	r0,_TRAP(r1)		/* register set saved */
+	b	sys_clone_with_pids
+
 	.globl	ppc_swapcontext
 ppc_swapcontext:
 	SAVE_NVGPRS(r1)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 43e0734..39fa316 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -320,6 +320,11 @@ _GLOBAL(ppc_clone)
 	bl	.sys_clone
 	b	syscall_exit
 
+_GLOBAL(ppc_clone_with_pids)
+	bl	.save_nvgprs
+	bl	.sys_clone_with_pids
+	b	syscall_exit
+
 _GLOBAL(ppc32_swapcontext)
 	bl	.save_nvgprs
 	bl	.compat_sys_swapcontext
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 892a9f2..5ac4470 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -898,6 +898,24 @@ int sys_clone(unsigned long clone_flags, unsigned long usp,
  	return do_fork(clone_flags, usp, regs, 0, parent_tidp, child_tidp);
 }
 
+int sys_clone_with_pids(unsigned long clone_flags, unsigned long usp,
+	      int __user *parent_tidp, void __user *child_threadptr,
+	      int __user *child_tidp, void __user *upid_setp,
+	      struct pt_regs *regs)
+{
+	CHECK_FULL_REGS(regs);
+	if (usp == 0)
+		usp = regs->gpr[1];
+#ifdef CONFIG_PPC64
+	if (test_thread_flag(TIF_32BIT)) {
+		parent_tidp = TRUNC_PTR(parent_tidp);
+		child_tidp = TRUNC_PTR(child_tidp);
+	}
+#endif
+ 	return do_fork_with_pids(clone_flags, usp, regs, 0,
+				 parent_tidp, child_tidp, upid_setp);
+}
+
 int sys_fork(unsigned long p1, unsigned long p2, unsigned long p3,
 	     unsigned long p4, unsigned long p5, unsigned long p6,
 	     struct pt_regs *regs)
-- 
1.6.0.6

Re: [PATCH 1/9] checkpoint: fix integer-pointer conversion warnings

[Serge E. Hallyn]

Quoting Nathan Lynch (ntl@pobox.com):
>   CC      checkpoint/process.o
> checkpoint/process.c: In function ‘checkpoint_task_struct’:
> checkpoint/process.c:97: warning: assignment makes integer from pointer without a cast
> checkpoint/process.c:98: warning: assignment makes integer from pointer without a cast
> checkpoint/process.c: In function ‘restore_task_struct’:
> checkpoint/process.c:475: warning: assignment makes pointer from integer without a cast
> checkpoint/process.c:476: warning: assignment makes pointer from integer without a cast
> 
> Signed-off-by: Nathan Lynch <ntl@pobox.com>
> ---
>  checkpoint/process.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/checkpoint/process.c b/checkpoint/process.c
> index 41566db..b7f4a43 100644
> --- a/checkpoint/process.c
> +++ b/checkpoint/process.c
> @@ -94,8 +94,8 @@ static int checkpoint_task_struct(struct ckpt_ctx *ctx, struct task_struct *t)
>  		h->exit_signal = t->exit_signal;
>  		h->pdeath_signal = t->pdeath_signal;
>  
> -		h->set_child_tid = t->set_child_tid;
> -		h->clear_child_tid = t->clear_child_tid;
> +		h->set_child_tid = (unsigned long)t->set_child_tid;
> +		h->clear_child_tid = (unsigned long)t->clear_child_tid;

But it's defined as __u64 in checkpoint_hdr.h.  So on 32-bit this cast
won't be right, right?

But a major ack to shutting this thing up.

thanks,
-serge

>  		save_task_robust_futex_list(h, t);
>  	}
>  
> @@ -472,8 +472,8 @@ static int restore_task_struct(struct ckpt_ctx *ctx)
>  		t->exit_signal = h->exit_signal;
>  		t->pdeath_signal = h->pdeath_signal;
>  
> -		t->set_child_tid = h->set_child_tid;
> -		t->clear_child_tid = h->clear_child_tid;
> +		t->set_child_tid = (void __user *)h->set_child_tid;
> +		t->clear_child_tid = (void __user *)h->clear_child_tid;
>  		restore_task_robust_futex_list(h);
>  	}
>  
> -- 
> 1.6.0.6
> 
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 1/9] checkpoint: fix integer-pointer conversion warnings

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Nathan Lynch (ntl@pobox.com):
>>   CC      checkpoint/process.o
>> checkpoint/process.c: In function ‘checkpoint_task_struct’:
>> checkpoint/process.c:97: warning: assignment makes integer from pointer without a cast
>> checkpoint/process.c:98: warning: assignment makes integer from pointer without a cast
>> checkpoint/process.c: In function ‘restore_task_struct’:
>> checkpoint/process.c:475: warning: assignment makes pointer from integer without a cast
>> checkpoint/process.c:476: warning: assignment makes pointer from integer without a cast
>>
>> Signed-off-by: Nathan Lynch <ntl@pobox.com>
>> ---
>>  checkpoint/process.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/checkpoint/process.c b/checkpoint/process.c
>> index 41566db..b7f4a43 100644
>> --- a/checkpoint/process.c
>> +++ b/checkpoint/process.c
>> @@ -94,8 +94,8 @@ static int checkpoint_task_struct(struct ckpt_ctx *ctx, struct task_struct *t)
>>  		h->exit_signal = t->exit_signal;
>>  		h->pdeath_signal = t->pdeath_signal;
>>  
>> -		h->set_child_tid = t->set_child_tid;
>> -		h->clear_child_tid = t->clear_child_tid;
>> +		h->set_child_tid = (unsigned long)t->set_child_tid;
>> +		h->clear_child_tid = (unsigned long)t->clear_child_tid;
> 
> But it's defined as __u64 in checkpoint_hdr.h.  So on 32-bit this cast
> won't be right, right?

Should be harmless.

> 
> But a major ack to shutting this thing up.

Definitely. In fact, it's addressed already in the PGID patch :)

Oren.

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 0/9] powerpc checkpoint/restart kernel support

[Oren Laadan]

Thanks Nathan,

I'll apply the general fixes right away.

I'm about to push ckpt-v18-rc1 today. Would you like that to include
the remaining patches as well ?

Oren.


Nathan Lynch wrote:
> Patches 1-3 fix general issues and can be applied immediately.
> 
> Patches 4-8 implement the checkpoint and restart syscalls and
> architecture callbacks.
> 
> Patch 9 implements the clone_with_pids syscall, but I think the shape
> of that interface is yet to be finalized, so I guess it's FYI for now.
> 
> Simple single task checkpoint and restart work, as does
> clone_with_pids, apparently.  Multiple task restart fails; the
> coordinator task appears to be receiving a signal during
> wait_all_tasks_finish() -> wait_for_completion_interruptible() for
> some reason I am still trying to tracking down.
> 
> Nathan Lynch (9):
>   checkpoint: fix integer-pointer conversion warnings
>   fix trivial build breaks in compat code
>   futex.h: fix compat builds
>   powerpc: reserve checkpoint arch identifiers
>   powerpc: provide APIs for validating and updating DABR
>   powerpc: checkpoint/restart implementation
>   powerpc: wire up checkpoint and restart syscalls
>   powerpc: enable checkpoint support in Kconfig
>   powerpc: clone_with_pids implementation
> 
>  arch/powerpc/Kconfig                      |    3 +
>  arch/powerpc/include/asm/Kbuild           |    1 +
>  arch/powerpc/include/asm/checkpoint_hdr.h |   22 ++
>  arch/powerpc/include/asm/ptrace.h         |    7 +
>  arch/powerpc/include/asm/syscalls.h       |    4 +
>  arch/powerpc/include/asm/systbl.h         |    3 +
>  arch/powerpc/include/asm/unistd.h         |    5 +-
>  arch/powerpc/kernel/entry_32.S            |    8 +
>  arch/powerpc/kernel/entry_64.S            |    5 +
>  arch/powerpc/kernel/process.c             |   18 +
>  arch/powerpc/kernel/ptrace.c              |   88 ++++--
>  arch/powerpc/mm/Makefile                  |    1 +
>  arch/powerpc/mm/checkpoint.c              |  531 +++++++++++++++++++++++++++++
>  checkpoint/process.c                      |   14 +-
>  include/linux/checkpoint_hdr.h            |    2 +
>  include/linux/futex.h                     |    4 +
>  16 files changed, 679 insertions(+), 37 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/checkpoint_hdr.h
>  create mode 100644 arch/powerpc/mm/checkpoint.c
> 

Re: [PATCH 3/9] futex.h: fix compat builds

[Oren Laadan]

Nathan Lynch wrote:
> With any build where CONFIG_COMPAT=y:
> 
> checkpoint/process.c: In function 'save_task_robust_futex_list':
> checkpoint/process.c:37: error: implicit declaration of function 'ptr_to_compat'
> checkpoint/process.c:38: error: dereferencing pointer to incomplete type
> checkpoint/process.c: In function 'restore_task_robust_futex_list':
> checkpoint/process.c:54: error: implicit declaration of function 'compat_ptr'
> checkpoint/process.c:54: warning: assignment makes pointer from integer without a cast
> checkpoint/process.c:55: error: implicit declaration of function 'do_compat_set_robust_list'
> 
> ptr_to_compat et al need compat.h; however, futex.h is exported while
> compat.h is not.  So the include is guarded by #ifdef __KERNEL__.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Including futex.h does not require compat.h. Rather, it's the code
in checkpoint/process.c that uses ptr_to_compat() et al. Is there a
reason not to include compat.h from checkpoint/process.c instead ?

Oren.

Re: [PATCH 1/9] checkpoint: fix integer-pointer conversion warnings

[Serge E. Hallyn]

Quoting Oren Laadan (orenl@librato.com):
> 
> 
> Serge E. Hallyn wrote:
> > Quoting Nathan Lynch (ntl@pobox.com):
> >>   CC      checkpoint/process.o
> >> checkpoint/process.c: In function ‘checkpoint_task_struct’:
> >> checkpoint/process.c:97: warning: assignment makes integer from pointer without a cast
> >> checkpoint/process.c:98: warning: assignment makes integer from pointer without a cast
> >> checkpoint/process.c: In function ‘restore_task_struct’:
> >> checkpoint/process.c:475: warning: assignment makes pointer from integer without a cast
> >> checkpoint/process.c:476: warning: assignment makes pointer from integer without a cast
> >>
> >> Signed-off-by: Nathan Lynch <ntl@pobox.com>
> >> ---
> >>  checkpoint/process.c |    8 ++++----
> >>  1 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/checkpoint/process.c b/checkpoint/process.c
> >> index 41566db..b7f4a43 100644
> >> --- a/checkpoint/process.c
> >> +++ b/checkpoint/process.c
> >> @@ -94,8 +94,8 @@ static int checkpoint_task_struct(struct ckpt_ctx *ctx, struct task_struct *t)
> >>  		h->exit_signal = t->exit_signal;
> >>  		h->pdeath_signal = t->pdeath_signal;
> >>  
> >> -		h->set_child_tid = t->set_child_tid;
> >> -		h->clear_child_tid = t->clear_child_tid;
> >> +		h->set_child_tid = (unsigned long)t->set_child_tid;
> >> +		h->clear_child_tid = (unsigned long)t->clear_child_tid;
> > 
> > But it's defined as __u64 in checkpoint_hdr.h.  So on 32-bit this cast
> > won't be right, right?
> 
> Should be harmless.
> 
> > 
> > But a major ack to shutting this thing up.
> 
> Definitely. In fact, it's addressed already in the PGID patch :)

Right, I thought it looked familiar :)
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 3/9] futex.h: fix compat builds

[Nathan Lynch]

On Wed, 2009-09-16 at 11:36 -0400, Oren Laadan wrote:
> 
> Nathan Lynch wrote:
> > With any build where CONFIG_COMPAT=y:
> > 
> > checkpoint/process.c: In function 'save_task_robust_futex_list':
> > checkpoint/process.c:37: error: implicit declaration of function 'ptr_to_compat'
> > checkpoint/process.c:38: error: dereferencing pointer to incomplete type
> > checkpoint/process.c: In function 'restore_task_robust_futex_list':
> > checkpoint/process.c:54: error: implicit declaration of function 'compat_ptr'
> > checkpoint/process.c:54: warning: assignment makes pointer from integer without a cast
> > checkpoint/process.c:55: error: implicit declaration of function 'do_compat_set_robust_list'
> > 
> > ptr_to_compat et al need compat.h; however, futex.h is exported while
> > compat.h is not.  So the include is guarded by #ifdef __KERNEL__.
> > 
> > Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
> 
> Including futex.h does not require compat.h. Rather, it's the code
> in checkpoint/process.c that uses ptr_to_compat() et al. Is there a
> reason not to include compat.h from checkpoint/process.c instead ?

Oh my, you're right.  I had this hanging around from an older tree where
futex.h did need it, but it's obviously not the right fix now.  The
include should go in checkpoint/process.c.

Re: [PATCH 0/9] powerpc checkpoint/restart kernel support

[Nathan Lynch]

On Wed, 2009-09-16 at 11:11 -0400, Oren Laadan wrote:
> Thanks Nathan,
> 
> I'll apply the general fixes right away.
> 
> I'm about to push ckpt-v18-rc1 today. Would you like that to include
> the remaining patches as well ?

Yes, please.  Thank you.