* [PATCH 0/3] c/r: Add s390 support @ 2009-03-03 15:56 Dan Smith 2009-03-03 15:56 ` [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs Dan Smith ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Dan Smith @ 2009-03-03 15:56 UTC (permalink / raw) To: containers; +Cc: linux-kernel This set adds checkpoint/restart support for s390. It is to be applied on top of Oren's ckpt-v13 set[1] and has been tested with the single and multi- process tests in user-cr. 1: http://lkml.org/lkml/2009/1/27/227 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs 2009-03-03 15:56 [PATCH 0/3] c/r: Add s390 support Dan Smith @ 2009-03-03 15:56 ` Dan Smith 2009-03-03 16:08 ` Dave Hansen 2009-03-03 15:56 ` [PATCH 2/3] c/r: Add CR_COPY() macro (v3) Dan Smith 2009-03-03 15:56 ` [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) Dan Smith 2 siblings, 1 reply; 18+ messages in thread From: Dan Smith @ 2009-03-03 15:56 UTC (permalink / raw) To: containers; +Cc: linux-kernel, linux-s390 We need to use this value in the checkpoint/restart code and would like to have a constant instead of a magic '3'. Signed-off-by: Dan Smith <danms@us.ibm.com> Cc: linux-s390@vger.kernel.org --- arch/s390/include/asm/ptrace.h | 2 ++ arch/s390/kernel/compat_ptrace.h | 3 ++- 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h index 5396f9f..d5b0781 100644 --- a/arch/s390/include/asm/ptrace.h +++ b/arch/s390/include/asm/ptrace.h @@ -172,6 +172,8 @@ #define NUM_CRS 16 #define NUM_ACRS 16 +#define NUM_CR_WORDS 3 + #define FPR_SIZE 8 #define FPC_SIZE 4 #define FPC_PAD_SIZE 4 /* gcc insists on aligning the fpregs */ diff --git a/arch/s390/kernel/compat_ptrace.h b/arch/s390/kernel/compat_ptrace.h index a2be3a9..123dd66 100644 --- a/arch/s390/kernel/compat_ptrace.h +++ b/arch/s390/kernel/compat_ptrace.h @@ -1,10 +1,11 @@ #ifndef _PTRACE32_H #define _PTRACE32_H +#include <asm/ptrace.h> /* needed for NUM_CR_WORDS */ #include "compat_linux.h" /* needed for psw_compat_t */ typedef struct { - __u32 cr[3]; + __u32 cr[NUM_CR_WORDS]; } per_cr_words32; typedef struct { -- 1.6.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs 2009-03-03 15:56 ` [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs Dan Smith @ 2009-03-03 16:08 ` Dave Hansen 2009-03-04 0:56 ` Dan Smith 0 siblings, 1 reply; 18+ messages in thread From: Dave Hansen @ 2009-03-03 16:08 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-s390, linux-kernel On Tue, 2009-03-03 at 10:56 -0500, Dan Smith wrote: > We need to use this value in the checkpoint/restart code and would like to > have a constant instead of a magic '3'. This doesn't do a lot of good unless it gets used in the base s390 code. Otherwise, why not just define it near its use in the c/r code? -- Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs 2009-03-03 16:08 ` Dave Hansen @ 2009-03-04 0:56 ` Dan Smith 2009-03-04 0:59 ` Dave Hansen 0 siblings, 1 reply; 18+ messages in thread From: Dan Smith @ 2009-03-04 0:56 UTC (permalink / raw) To: Dave Hansen; +Cc: containers, linux-s390, linux-kernel DH> This doesn't do a lot of good unless it gets used in the base s390 DH> code. Otherwise, why not just define it near its use in the c/r DH> code? I intended to replace all uses, but I just found one place I missed, but I think that's it. If you know of other places that it's used in the s390 code, other than in compat_ptrace.h and ptrace.h please point them out. -- Dan Smith IBM Linux Technology Center email: danms@us.ibm.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs 2009-03-04 0:56 ` Dan Smith @ 2009-03-04 0:59 ` Dave Hansen 0 siblings, 0 replies; 18+ messages in thread From: Dave Hansen @ 2009-03-04 0:59 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-s390, linux-kernel On Tue, 2009-03-03 at 16:56 -0800, Dan Smith wrote: > DH> This doesn't do a lot of good unless it gets used in the base s390 > DH> code. Otherwise, why not just define it near its use in the c/r > DH> code? > > I intended to replace all uses, but I just found one place I missed, > but I think that's it. If you know of other places that it's used in > the s390 code, other than in compat_ptrace.h and ptrace.h please point > them out. My only knowledge of where else it was used came from you. :) -- Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-03 15:56 [PATCH 0/3] c/r: Add s390 support Dan Smith 2009-03-03 15:56 ` [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs Dan Smith @ 2009-03-03 15:56 ` Dan Smith 2009-03-03 16:22 ` Dave Hansen 2009-03-04 20:01 ` Nathan Lynch 2009-03-03 15:56 ` [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) Dan Smith 2 siblings, 2 replies; 18+ messages in thread From: Dan Smith @ 2009-03-03 15:56 UTC (permalink / raw) To: containers; +Cc: linux-kernel As suggested by Dave[1], this provides us a way to make the copy-in and copy-out processes symmetric. CR_COPY_ARRAY() provides us a way to do the same thing but for arrays. It's not critical, but it helps us unify the checkpoint and restart paths for some things. Changelog: Feb 27: . Changed CR_COPY() to use assignment, eliminating the need for the CR_COPY_BIT() macro . Add CR_COPY_ARRAY() macro to help copying register arrays, etc . Move the macro definitions inside the CR #ifdef Feb 25: . Changed WARN_ON() to BUILD_BUG_ON() Signed-off-by: Dan Smith <danms@us.ibm.com> 1: https://lists.linux-foundation.org/pipermail/containers/2009-February/015821.html (all the way at the bottom) --- include/linux/checkpoint.h | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h index 217cf6e..735668f 100644 --- a/include/linux/checkpoint.h +++ b/include/linux/checkpoint.h @@ -142,6 +142,26 @@ static inline void __task_deny_checkpointing(struct task_struct *task, #define task_deny_checkpointing(p) \ __task_deny_checkpointing(p, __FILE__, __LINE__) +#define CR_CPT 1 +#define CR_RST 2 + +#define CR_COPY(op, a, b) \ + do { \ + if (op == CR_CPT) \ + a = b; \ + else \ + b = a; \ + } while (0); + +#define CR_COPY_ARRAY(op, a, b, count) \ + do { \ + BUILD_BUG_ON(sizeof(*a) != sizeof(*b)); \ + if (op == CR_CPT) \ + memcpy(a, b, count * sizeof(*a)); \ + else \ + memcpy(b, a, count * sizeof(*a)); \ + } while (0); + #else static inline void task_deny_checkpointing(struct task_struct *task) {} -- 1.6.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-03 15:56 ` [PATCH 2/3] c/r: Add CR_COPY() macro (v3) Dan Smith @ 2009-03-03 16:22 ` Dave Hansen 2009-03-04 0:57 ` Dan Smith 2009-03-04 20:01 ` Nathan Lynch 1 sibling, 1 reply; 18+ messages in thread From: Dave Hansen @ 2009-03-03 16:22 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-kernel On Tue, 2009-03-03 at 10:56 -0500, Dan Smith wrote: > As suggested by Dave[1], this provides us a way to make the copy-in and > copy-out processes symmetric. CR_COPY_ARRAY() provides us a way to do > the same thing but for arrays. It's not critical, but it helps us unify > the checkpoint and restart paths for some things. Did you convince Nathan that this ends up being a good idea? -- Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-03 16:22 ` Dave Hansen @ 2009-03-04 0:57 ` Dan Smith 2009-03-04 1:00 ` Dave Hansen 0 siblings, 1 reply; 18+ messages in thread From: Dan Smith @ 2009-03-04 0:57 UTC (permalink / raw) To: Dave Hansen; +Cc: containers, linux-kernel DH> Did you convince Nathan that this ends up being a good idea? Technically he hasn't seen this version, but my hopes are not high that he will change his mind. If the feedback is that they're not liked, I'll happily remove them. -- Dan Smith IBM Linux Technology Center email: danms@us.ibm.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 0:57 ` Dan Smith @ 2009-03-04 1:00 ` Dave Hansen 2009-03-04 15:05 ` Serge E. Hallyn 2009-03-04 19:53 ` Nathan Lynch 0 siblings, 2 replies; 18+ messages in thread From: Dave Hansen @ 2009-03-04 1:00 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-kernel On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: > DH> Did you convince Nathan that this ends up being a good idea? > > Technically he hasn't seen this version, but my hopes are not high > that he will change his mind. If the feedback is that they're not > liked, I'll happily remove them. I just figure if Nathan feels that strongly that we'll encounter more people who feel even more so. So, I was curious if he changed his mind somehow. -- Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 1:00 ` Dave Hansen @ 2009-03-04 15:05 ` Serge E. Hallyn 2009-03-18 7:51 ` Oren Laadan 2009-03-04 19:53 ` Nathan Lynch 1 sibling, 1 reply; 18+ messages in thread From: Serge E. Hallyn @ 2009-03-04 15:05 UTC (permalink / raw) To: Dave Hansen; +Cc: Dan Smith, containers, linux-kernel Quoting Dave Hansen (dave@linux.vnet.ibm.com): > On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: > > DH> Did you convince Nathan that this ends up being a good idea? > > > > Technically he hasn't seen this version, but my hopes are not high > > that he will change his mind. If the feedback is that they're not > > liked, I'll happily remove them. > > I just figure if Nathan feels that strongly that we'll encounter more > people who feel even more so. So, I was curious if he changed his mind > somehow. I maintain however that two strong advantages of moving the checkpoint and restart of simple registers etc into a single function are: 1. we won't forget to add (or accidentally lose) one or the other 2. any actual special handling at checkpoint or restart, like the loading of access registers at restart on s390x, stand out -serge ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 15:05 ` Serge E. Hallyn @ 2009-03-18 7:51 ` Oren Laadan 2009-03-18 13:43 ` Serge E. Hallyn 0 siblings, 1 reply; 18+ messages in thread From: Oren Laadan @ 2009-03-18 7:51 UTC (permalink / raw) To: Serge E. Hallyn; +Cc: Dave Hansen, containers, Dan Smith, linux-kernel Serge E. Hallyn wrote: > Quoting Dave Hansen (dave@linux.vnet.ibm.com): >> On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: >>> DH> Did you convince Nathan that this ends up being a good idea? >>> >>> Technically he hasn't seen this version, but my hopes are not high >>> that he will change his mind. If the feedback is that they're not >>> liked, I'll happily remove them. >> I just figure if Nathan feels that strongly that we'll encounter more >> people who feel even more so. So, I was curious if he changed his mind >> somehow. > > I maintain however that two strong advantages of moving the checkpoint > and restart of simple registers etc into a single function are: > > 1. we won't forget to add (or accidentally lose) one or the > other > 2. any actual special handling at checkpoint or restart, like > the loading of access registers at restart on s390x, > stand out > I, too, think that this scheme is elegant, and at the same time I, too, think that it obfuscates the code. Since I only touch arch-dependent code only if I really really must, I don't have strong opinion about it ;) However, a problem with this scheme is that checkpoint and restart are not fully symmetric -- on restart we must sanitize the input data before restoring the registers to that data. I'm not familiar with s390, but it is likely that by not doing so we create a security issue. Oren. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-18 7:51 ` Oren Laadan @ 2009-03-18 13:43 ` Serge E. Hallyn 0 siblings, 0 replies; 18+ messages in thread From: Serge E. Hallyn @ 2009-03-18 13:43 UTC (permalink / raw) To: Oren Laadan; +Cc: Dave Hansen, containers, Dan Smith, linux-kernel Quoting Oren Laadan (orenl@cs.columbia.edu): > > > Serge E. Hallyn wrote: > > Quoting Dave Hansen (dave@linux.vnet.ibm.com): > >> On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: > >>> DH> Did you convince Nathan that this ends up being a good idea? > >>> > >>> Technically he hasn't seen this version, but my hopes are not high > >>> that he will change his mind. If the feedback is that they're not > >>> liked, I'll happily remove them. > >> I just figure if Nathan feels that strongly that we'll encounter more > >> people who feel even more so. So, I was curious if he changed his mind > >> somehow. > > > > I maintain however that two strong advantages of moving the checkpoint > > and restart of simple registers etc into a single function are: > > > > 1. we won't forget to add (or accidentally lose) one or the > > other > > 2. any actual special handling at checkpoint or restart, like > > the loading of access registers at restart on s390x, > > stand out > > > > I, too, think that this scheme is elegant, and at the same time I, too, > think that it obfuscates the code. Since I only touch arch-dependent code > only if I really really must, I don't have strong opinion about it ;) > > However, a problem with this scheme is that checkpoint and restart > are not fully symmetric -- on restart we must sanitize the input data > before restoring the registers to that data. I'm not familiar with > s390, but it is likely that by not doing so we create a security issue. > > Oren. But that's exactly why I think CR_COPY() helps - the sanitation is explicit next to some boring CR_COPY()s. It becomes clearer that it is being done. Anyway we've got plenty of other, bigger hurdles to clear, so while I do have a strong opinion, I'm not planning on pushing hard either way. thanks, -serge ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 1:00 ` Dave Hansen 2009-03-04 15:05 ` Serge E. Hallyn @ 2009-03-04 19:53 ` Nathan Lynch 2009-03-04 20:18 ` Dave Hansen 1 sibling, 1 reply; 18+ messages in thread From: Nathan Lynch @ 2009-03-04 19:53 UTC (permalink / raw) To: Dave Hansen; +Cc: Dan Smith, containers, linux-kernel On Tue, 03 Mar 2009 17:00:37 -0800 Dave Hansen <dave@linux.vnet.ibm.com> wrote: > On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: > > DH> Did you convince Nathan that this ends up being a good idea? > > > > Technically he hasn't seen this version, but my hopes are not high > > that he will change his mind. If the feedback is that they're not > > liked, I'll happily remove them. > > I just figure if Nathan feels that strongly that we'll encounter more > people who feel even more so. So, I was curious if he changed his mind > somehow. No, not really, sorry. I understand why it's nice for the developer to have this sort of helper, but I don't think it's nice for someone trying to review or debug the code. Surely discussing these macros has already consumed more developer time than they would ever save? :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 19:53 ` Nathan Lynch @ 2009-03-04 20:18 ` Dave Hansen 0 siblings, 0 replies; 18+ messages in thread From: Dave Hansen @ 2009-03-04 20:18 UTC (permalink / raw) To: Nathan Lynch; +Cc: containers, Dan Smith, linux-kernel On Wed, 2009-03-04 at 13:53 -0600, Nathan Lynch wrote: > On Tue, 03 Mar 2009 17:00:37 -0800 > Dave Hansen <dave@linux.vnet.ibm.com> wrote: > > > On Tue, 2009-03-03 at 16:57 -0800, Dan Smith wrote: > > > DH> Did you convince Nathan that this ends up being a good idea? > > > > > > Technically he hasn't seen this version, but my hopes are not high > > > that he will change his mind. If the feedback is that they're not > > > liked, I'll happily remove them. > > > > I just figure if Nathan feels that strongly that we'll encounter more > > people who feel even more so. So, I was curious if he changed his mind > > somehow. > > No, not really, sorry. > > I understand why it's nice for the developer to have this sort of > helper, but I don't think it's nice for someone trying to review or > debug the code. That's funny. I've only reviewed and debugged these things, but I don't think I've actually written any code that would have used these macros! As someone trying to debug and review, I love how this looks. It gets the point across much more clearly about what is going on to me as a reviewer and I appreciate that. memcpy()s contain a lot of gunk that my brain can't parse easily, but this is rather clean, and it *HALVES* the number of lines of code I have to look at. > Surely discussing these macros has already consumed more developer time > than they would ever save? :) That's exactly my point. We're not trying to save development time here at all. My argument is that this reduces the maintenance and review burden. -- Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-03 15:56 ` [PATCH 2/3] c/r: Add CR_COPY() macro (v3) Dan Smith 2009-03-03 16:22 ` Dave Hansen @ 2009-03-04 20:01 ` Nathan Lynch 2009-03-04 20:18 ` Dan Smith 1 sibling, 1 reply; 18+ messages in thread From: Nathan Lynch @ 2009-03-04 20:01 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-kernel Hi Dan. Dan Smith <danms@us.ibm.com> wrote: > +#define CR_CPT 1 > +#define CR_RST 2 > + > +#define CR_COPY(op, a, b) \ > + do { \ > + if (op == CR_CPT) \ > + a = b; \ > + else \ > + b = a; \ > + } while (0); Drop the semicolon ^ > + > +#define CR_COPY_ARRAY(op, a, b, count) \ > + do { \ > + BUILD_BUG_ON(sizeof(*a) != sizeof(*b)); \ > + if (op == CR_CPT) \ > + memcpy(a, b, count * sizeof(*a)); \ > + else \ > + memcpy(b, a, count * sizeof(*a)); \ > + } while (0); > + You might want to employ __must_be_array() or similar to catch misuse. Misuse might also be prevented by providing some documentation :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] c/r: Add CR_COPY() macro (v3) 2009-03-04 20:01 ` Nathan Lynch @ 2009-03-04 20:18 ` Dan Smith 0 siblings, 0 replies; 18+ messages in thread From: Dan Smith @ 2009-03-04 20:18 UTC (permalink / raw) To: Nathan Lynch; +Cc: containers, linux-kernel NL> Drop the semicolon ^ Thanks. NL> You might want to employ __must_be_array() or similar to catch NL> misuse. Ooh, nice. NL> Misuse might also be prevented by providing some documentation :) Agreed. Thanks! -- Dan Smith IBM Linux Technology Center email: danms@us.ibm.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) 2009-03-03 15:56 [PATCH 0/3] c/r: Add s390 support Dan Smith 2009-03-03 15:56 ` [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs Dan Smith 2009-03-03 15:56 ` [PATCH 2/3] c/r: Add CR_COPY() macro (v3) Dan Smith @ 2009-03-03 15:56 ` Dan Smith 2009-03-03 22:40 ` Serge E. Hallyn 2 siblings, 1 reply; 18+ messages in thread From: Dan Smith @ 2009-03-03 15:56 UTC (permalink / raw) To: containers; +Cc: linux-kernel, Serge E. Hallyn Implement the s390 arch-specific checkpoint/restart helpers. This is on top of Oren Laadan's c/r code. With these, I am able to checkpoint and restart simple programs as per Oren's patch intro. While on x86 I never had to freeze a single task to checkpoint it, on s390 I do need to. That is a prereq for consistent snapshots (esp with multiple processes) anyway so I don't see that as a problem. Changelog: Feb 27: . Add checkpoint_s390.h . Fixed up save and restore of PSW, with the non-address bits properly masked out Feb 25: . Make checkpoint_hdr.h safe for inclusion in userspace . Replace comment about vsdo code . Add comment about restoring access registers . Write and read an empty cr_hdr_head_arch record to appease code (mktree) that expects it to be there . Utilize NUM_CR_WORDS in checkpoint_hdr.h Feb 24: . Use CR_COPY() to unify the un/loading of cpu and mm state . Fix fprs definition in cr_hdr_cpu . Remove debug WARN_ON() from checkpoint.c Feb 23: . Macro-ize the un/packing of trace flags . Fix the crash when externally-linked . Break out the restart functions into restart.c . Remove unneeded s390_enable_sie() call Jan 30: . Switched types in cr_hdr_cpu to __u64 etc. (Per Oren suggestion) . Replaced direct inclusion of structs in cr_hdr_cpu with the struct members. (Per Oren suggestion) . Also ended up adding a bunch of new things into restart (mm_segment, ksp, etc) in vain attempt to get code using fpu to not segfault after restart. Signed-off-by: Serge E. Hallyn <serue@us.ibm.com> Signed-off-by: Dan Smith <danms@us.ibm.com> --- arch/s390/include/asm/checkpoint_hdr.h | 88 ++++++++++++++++++++++ arch/s390/include/asm/unistd.h | 4 +- arch/s390/kernel/compat_wrapper.S | 12 +++ arch/s390/kernel/syscalls.S | 2 + arch/s390/mm/Makefile | 1 + arch/s390/mm/checkpoint.c | 125 ++++++++++++++++++++++++++++++++ arch/s390/mm/checkpoint_s390.h | 22 ++++++ arch/s390/mm/restart.c | 81 +++++++++++++++++++++ checkpoint/Kconfig | 2 +- 9 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 arch/s390/include/asm/checkpoint_hdr.h create mode 100644 arch/s390/mm/checkpoint.c create mode 100644 arch/s390/mm/checkpoint_s390.h create mode 100644 arch/s390/mm/restart.c diff --git a/arch/s390/include/asm/checkpoint_hdr.h b/arch/s390/include/asm/checkpoint_hdr.h new file mode 100644 index 0000000..0a405c2 --- /dev/null +++ b/arch/s390/include/asm/checkpoint_hdr.h @@ -0,0 +1,88 @@ +#ifndef __ASM_S390_CKPT_HDR_H +#define __ASM_S390_CKPT_HDR_H +/* + * Checkpoint/restart - architecture specific headers s/390 + * + * Copyright IBM Corp. 2009 + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of the Linux + * distribution for more details. + */ + +#include <linux/types.h> +#include <asm/ptrace.h> + +#ifdef __KERNEL__ +#include <asm/processor.h> +#else +#include <sys/user.h> +#endif + +#ifdef __s390x__ + +/* + * Notes + * NUM_GPRS defined in <asm/ptrace.h> to be 16 + * NUM_FPRS defined in <asm/ptrace.h> to be 16 + * NUM_APRS defined in <asm/ptrace.h> to be 16 + * NUM_CR_WORDS defined in <asm/ptrace.h> to be 3 + */ +struct cr_hdr_cpu { + __u64 args[1]; + __u64 gprs[NUM_GPRS]; + __u64 orig_gpr2; + __u16 svcnr; + __u16 ilc; + __u32 acrs[NUM_ACRS]; + __u64 ieee_instruction_pointer; + + /* psw_t */ + __u64 psw_t_mask; + __u64 psw_t_addr; + + /* s390_fp_regs_t */ + __u32 fpc; + union { + float f; + double d; + __u64 ui; + struct { + __u32 fp_hi; + __u32 fp_lo; + } fp; + } fprs[NUM_FPRS]; + + /* per_struct */ + __u64 per_control_regs[NUM_CR_WORDS]; + __u64 starting_addr; + __u64 ending_addr; + __u64 address; + __u16 perc_atmid; + __u8 access_id; + __u8 single_step; + __u8 instruction_fetch; +}; + +struct cr_hdr_mm_context { + unsigned long vdso_base; + int noexec; + int has_pgste; + int alloc_pgste; + unsigned long asce_bits; + unsigned long asce_limit; +}; + +struct cr_hdr_head_arch { +}; + +#ifdef __KERNEL__ +/* Functions for copying to/from the header structs */ +extern void cr_s390_regs(int op, struct cr_hdr_cpu *hh, struct task_struct *t); +extern void cr_s390_mm(int op, struct cr_hdr_mm_context *hh, + struct mm_struct *mm); +#endif + +#endif /* __s390x__ */ + +#endif /* __ASM_S390_CKPT_HDR__H */ diff --git a/arch/s390/include/asm/unistd.h b/arch/s390/include/asm/unistd.h index c8ad350..ffe64a0 100644 --- a/arch/s390/include/asm/unistd.h +++ b/arch/s390/include/asm/unistd.h @@ -265,7 +265,9 @@ #define __NR_pipe2 325 #define __NR_dup3 326 #define __NR_epoll_create1 327 -#define NR_syscalls 328 +#define __NR_checkpoint 328 +#define __NR_restart 329 +#define NR_syscalls 330 /* * There are some system calls that are not present on 64 bit, some diff --git a/arch/s390/kernel/compat_wrapper.S b/arch/s390/kernel/compat_wrapper.S index fc2c971..9546a81 100644 --- a/arch/s390/kernel/compat_wrapper.S +++ b/arch/s390/kernel/compat_wrapper.S @@ -1767,3 +1767,15 @@ sys_dup3_wrapper: sys_epoll_create1_wrapper: lgfr %r2,%r2 # int jg sys_epoll_create1 # branch to system call + + .globl sys_checkpoint_wrapper +sys_checkpoint_wrapper: + lgfr %r2,%r2 # pid_t + lgfr %r3,%r3 # int + llgfr %r4,%r4 # unsigned long + + .globl sys_restart_wrapper +sys_restart_wrapper: + lgfr %r2,%r2 # int + lgfr %r3,%r3 # int + llgfr %r4,%r4 # unsigned long diff --git a/arch/s390/kernel/syscalls.S b/arch/s390/kernel/syscalls.S index 2d61787..54316c8 100644 --- a/arch/s390/kernel/syscalls.S +++ b/arch/s390/kernel/syscalls.S @@ -336,3 +336,5 @@ SYSCALL(sys_inotify_init1,sys_inotify_init1,sys_inotify_init1_wrapper) SYSCALL(sys_pipe2,sys_pipe2,sys_pipe2_wrapper) /* 325 */ SYSCALL(sys_dup3,sys_dup3,sys_dup3_wrapper) SYSCALL(sys_epoll_create1,sys_epoll_create1,sys_epoll_create1_wrapper) +SYSCALL(sys_checkpoint,sys_checkpoint,sys_checkpoint_wrapper) +SYSCALL(sys_restart,sys_restart,sys_restart_wrapper) diff --git a/arch/s390/mm/Makefile b/arch/s390/mm/Makefile index 2a74581..040fbb7 100644 --- a/arch/s390/mm/Makefile +++ b/arch/s390/mm/Makefile @@ -6,3 +6,4 @@ obj-y := init.o fault.o extmem.o mmap.o vmem.o pgtable.o obj-$(CONFIG_CMM) += cmm.o obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o obj-$(CONFIG_PAGE_STATES) += page-states.o +obj-$(CONFIG_CHECKPOINT_RESTART) += checkpoint.o restart.o diff --git a/arch/s390/mm/checkpoint.c b/arch/s390/mm/checkpoint.c new file mode 100644 index 0000000..113c865 --- /dev/null +++ b/arch/s390/mm/checkpoint.c @@ -0,0 +1,125 @@ +/* + * Checkpoint/restart - architecture specific support for s390 + * + * Copyright IBM Corp. 2009 + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of the Linux + * distribution for more details. + */ + +#include <linux/checkpoint.h> +#include <linux/checkpoint_hdr.h> +#include <linux/kernel.h> +#include <asm/system.h> +#include <asm/pgtable.h> + +#include "checkpoint_s390.h" + +void cr_s390_regs(int op, struct cr_hdr_cpu *hh, struct task_struct *t) +{ + struct pt_regs *regs = task_pt_regs(t); + struct thread_struct *thr = &t->thread; + + /* Save the whole PSW to facilitate forensic debugging, but only + * restore the address portion to avoid letting userspace do + * bad things by manipulating its value. + */ + if (op == CR_CPT) { + CR_COPY(op, hh->psw_t_addr, regs->psw.addr); + } else { + regs->psw.addr &= ~PSW_ADDR_INSN; + regs->psw.addr |= hh->psw_t_addr; + } + + CR_COPY(op, hh->args[0], regs->args[0]); + CR_COPY(op, hh->orig_gpr2, regs->orig_gpr2); + CR_COPY(op, hh->svcnr, regs->svcnr); + CR_COPY(op, hh->ilc, regs->ilc); + CR_COPY(op, hh->ieee_instruction_pointer, + thr->ieee_instruction_pointer); + CR_COPY(op, hh->psw_t_mask, regs->psw.mask); + CR_COPY(op, hh->fpc, thr->fp_regs.fpc); + CR_COPY(op, hh->starting_addr, thr->per_info.starting_addr); + CR_COPY(op, hh->ending_addr, thr->per_info.ending_addr); + CR_COPY(op, hh->address, thr->per_info.lowcore.words.address); + CR_COPY(op, hh->perc_atmid, thr->per_info.lowcore.words.perc_atmid); + CR_COPY(op, hh->access_id, thr->per_info.lowcore.words.access_id); + CR_COPY(op, hh->single_step, thr->per_info.single_step); + CR_COPY(op, hh->instruction_fetch, thr->per_info.instruction_fetch); + + CR_COPY_ARRAY(op, hh->gprs, regs->gprs, NUM_GPRS); + CR_COPY_ARRAY(op, hh->fprs, thr->fp_regs.fprs, NUM_FPRS); + CR_COPY_ARRAY(op, hh->acrs, thr->acrs, NUM_ACRS); + CR_COPY_ARRAY(op, hh->per_control_regs, + thr->per_info.control_regs.words.cr, NUM_CR_WORDS); + +} + +void cr_s390_mm(int op, struct cr_hdr_mm_context *hh, struct mm_struct *mm) +{ + CR_COPY(op, hh->noexec, mm->context.noexec); + CR_COPY(op, hh->has_pgste, mm->context.has_pgste); + CR_COPY(op, hh->alloc_pgste, mm->context.alloc_pgste); + CR_COPY(op, hh->asce_bits, mm->context.asce_bits); + CR_COPY(op, hh->asce_limit, mm->context.asce_limit); +} + +int cr_write_thread(struct cr_ctx *ctx, struct task_struct *t) +{ + return 0; +} + +/* dump the cpu state and registers of a given task */ +int cr_write_cpu(struct cr_ctx *ctx, struct task_struct *t) +{ + struct cr_hdr h; + struct cr_hdr_cpu *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int ret; + + h.type = CR_HDR_CPU; + h.len = sizeof(*hh); + h.parent = task_pid_vnr(t); + + cr_s390_regs(CR_CPT, hh, t); + + ret = cr_write_obj(ctx, &h, hh); + cr_hbuf_put(ctx, sizeof(*hh)); + + return ret; +} + +/* Write an empty header since it is assumed to be there */ +int cr_write_head_arch(struct cr_ctx *ctx) +{ + struct cr_hdr h; + struct cr_hdr_head_arch *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int ret; + + h.type = CR_HDR_HEAD_ARCH; + h.len = sizeof(*hh); + h.parent = 0; + + ret = cr_write_obj(ctx, &h, &hh); + cr_hbuf_put(ctx, sizeof(*hh)); + + return ret; +} + +int cr_write_mm_context(struct cr_ctx *ctx, struct mm_struct *mm, int parent) +{ + struct cr_hdr h; + struct cr_hdr_mm_context *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int ret; + + h.type = CR_HDR_MM_CONTEXT; + h.len = sizeof(*hh); + h.parent = parent; + + cr_s390_mm(CR_CPT, hh, mm); + + ret = cr_write_obj(ctx, &h, hh); + cr_hbuf_put(ctx, sizeof(*hh)); + + return ret; +} diff --git a/arch/s390/mm/checkpoint_s390.h b/arch/s390/mm/checkpoint_s390.h new file mode 100644 index 0000000..52a5e6f --- /dev/null +++ b/arch/s390/mm/checkpoint_s390.h @@ -0,0 +1,22 @@ +/* + * Checkpoint/restart - architecture specific support for s390 + * + * Copyright IBM Corp. 2009 + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of the Linux + * distribution for more details. + */ + +#ifndef _S390_CHECKPOINT_H +#define _S390_CHECKPOINT_H + +#include <linux/checkpoint_hdr.h> +#include <linux/sched.h> +#include <linux/mm_types.h> + +extern void cr_s390_regs(int op, struct cr_hdr_cpu *hh, struct task_struct *t); +extern void cr_s390_mm(int op, struct cr_hdr_mm_context *hh, + struct mm_struct *mm); + +#endif /* _S390_CHECKPOINT_H */ diff --git a/arch/s390/mm/restart.c b/arch/s390/mm/restart.c new file mode 100644 index 0000000..7da4111 --- /dev/null +++ b/arch/s390/mm/restart.c @@ -0,0 +1,81 @@ +/* + * Checkpoint/restart - architecture specific support for s390 + * + * Copyright IBM Corp. 2009 + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file COPYING in the main directory of the Linux + * distribution for more details. + */ + +#include <linux/checkpoint.h> +#include <linux/checkpoint_hdr.h> +#include <linux/kernel.h> +#include <asm/system.h> +#include <asm/pgtable.h> + +#include "checkpoint_s390.h" + +int cr_read_thread(struct cr_ctx *ctx) +{ + return 0; +} + +int cr_read_cpu(struct cr_ctx *ctx) +{ + struct cr_hdr_cpu *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int parent, ret; + + parent = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_CPU); + if (parent < 0) { + ret = parent; + goto out; + } + ret = 0; + + cr_s390_regs(CR_RST, hh, current); + + /* s390 does not restore the access registers after a syscall, + * but does on a task switch. Since we're switching tasks (in + * a way), we need to replicate that behavior here. + */ + restore_access_regs(hh->acrs); +out: + cr_hbuf_put(ctx, sizeof(*hh)); + return ret; +} + +int cr_read_head_arch(struct cr_ctx *ctx) +{ + struct cr_hdr_head_arch *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int parent, ret = 0; + + parent = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_HEAD_ARCH); + if (parent < 0) + ret = parent; + + cr_hbuf_put(ctx, sizeof(*hh)); + + return ret; +} + + +int cr_read_mm_context(struct cr_ctx *ctx, struct mm_struct *mm, int rparent) +{ + struct cr_hdr_mm_context *hh = cr_hbuf_get(ctx, sizeof(*hh)); + int parent, ret = -EINVAL; + + parent = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_MM_CONTEXT); + if (parent < 0) { + ret = parent; + goto out; + } + if (parent != rparent) + goto out; + + cr_s390_mm(CR_RST, hh, mm); + ret = 0; + out: + cr_hbuf_put(ctx, sizeof(*hh)); + return ret; +} diff --git a/checkpoint/Kconfig b/checkpoint/Kconfig index ffaa635..cb1d29d 100644 --- a/checkpoint/Kconfig +++ b/checkpoint/Kconfig @@ -1,7 +1,7 @@ config CHECKPOINT_RESTART prompt "Enable checkpoint/restart (EXPERIMENTAL)" def_bool n - depends on X86_32 && EXPERIMENTAL + depends on (X86_32 || (S390 && 64BIT)) && EXPERIMENTAL help Application checkpoint/restart is the ability to save the state of a running application so that it can later resume -- 1.6.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) 2009-03-03 15:56 ` [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) Dan Smith @ 2009-03-03 22:40 ` Serge E. Hallyn 0 siblings, 0 replies; 18+ messages in thread From: Serge E. Hallyn @ 2009-03-03 22:40 UTC (permalink / raw) To: Dan Smith; +Cc: containers, linux-kernel Quoting Dan Smith (danms@us.ibm.com): > Implement the s390 arch-specific checkpoint/restart helpers. This > is on top of Oren Laadan's c/r code. Working nicely for me, thanks. -serge ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2009-03-18 13:44 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-03-03 15:56 [PATCH 0/3] c/r: Add s390 support Dan Smith 2009-03-03 15:56 ` [PATCH 1/3] s390: Expose a constant for the number of words representing the CRs Dan Smith 2009-03-03 16:08 ` Dave Hansen 2009-03-04 0:56 ` Dan Smith 2009-03-04 0:59 ` Dave Hansen 2009-03-03 15:56 ` [PATCH 2/3] c/r: Add CR_COPY() macro (v3) Dan Smith 2009-03-03 16:22 ` Dave Hansen 2009-03-04 0:57 ` Dan Smith 2009-03-04 1:00 ` Dave Hansen 2009-03-04 15:05 ` Serge E. Hallyn 2009-03-18 7:51 ` Oren Laadan 2009-03-18 13:43 ` Serge E. Hallyn 2009-03-04 19:53 ` Nathan Lynch 2009-03-04 20:18 ` Dave Hansen 2009-03-04 20:01 ` Nathan Lynch 2009-03-04 20:18 ` Dan Smith 2009-03-03 15:56 ` [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v7) Dan Smith 2009-03-03 22:40 ` Serge E. Hallyn
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).