Linux Container Development
 help / color / mirror / Atom feed
* [PATCH 0/8] checkpoint: improve handling of pending signals
@ 2010-07-13 15:44 Nathan Lynch
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Hi Oren,

The following series improves the support for checkpointing pending
signal state and (I think) is exhaustive.  These have been tested on
top of your ckpt-v22-dev branch on x86 and powerpc systems for the
last couple of weeks.

Nathan Lynch (8):
  checkpoint: model ckpt_siginfo on signalfd_siginfo
  checkpoint: add const qualifiers to siginfo handlers
  checkpoint: clean up pending posix timer signal handling
  checkpoint: clean up __SI_POLL siginfo save/restore
  checkpoint: clean up __SI_FAULT siginfo save/restore
  checkpoint: clean up __SI_CHLD siginfo save/restore
  checkpoint: __SI_KILL does not use si_ptr
  checkpoint: handle siginfo->si_code < 0

 include/linux/checkpoint_hdr.h |   29 +++++----
 kernel/signal.c                |  134 +++++++++++++++++++++++++++-------------
 2 files changed, 107 insertions(+), 56 deletions(-)

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

* [PATCH 1/8] checkpoint: model ckpt_siginfo on signalfd_siginfo
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
@ 2010-07-13 15:44   ` Nathan Lynch
       [not found]     ` <1279035864-10533-2-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
  2010-07-13 15:44   ` [PATCH 2/8] checkpoint: add const qualifiers to siginfo handlers Nathan Lynch
                     ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

ckpt_siginfo doesn't cover all of the possible siginfo fields; the
current code overloads some fields (such as si_uid for si_overrun),
which is confusing.  Signed-ness on some fields is not correct either.

signalfd_siginfo is a straightforward format for representing siginfo,
so copy it and update the signal checkpoint code to use the new
fields.  Although the ckpt_siginfo struct grows, there should be no
behavioral changes.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 include/linux/checkpoint_hdr.h |   28 ++++++++------
 kernel/signal.c                |   80 ++++++++++++++++++++--------------------
 2 files changed, 56 insertions(+), 52 deletions(-)

diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index c8383c0..b46d586 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -919,20 +919,24 @@ struct ckpt_hdr_sighand {
 	struct ckpt_sigaction action[0];
 } __attribute__((aligned(8)));
 
-#ifndef HAVE_ARCH_SIGINFO_T
 struct ckpt_siginfo {
-	__u32 signo;
-	__u32 _errno;
-	__u32 code;
-
-	__u32 pid;
-	__s32 uid;
-	__u32 sigval_int;
-	__u64 sigval_ptr;
-	__u64 utime;
-	__u64 stime;
+	__u32 csi_signo;
+	__s32 csi_errno;
+	__s32 csi_code;
+	__u32 csi_pid;
+	__u32 csi_uid;
+	__s32 csi_fd;
+	__u32 csi_tid;
+	__u32 csi_band;
+	__u32 csi_overrun;
+	__u32 csi_trapno;
+	__s32 csi_status;
+	__s32 csi_int;
+	__u64 csi_ptr;
+	__u64 csi_utime;
+	__u64 csi_stime;
+	__u64 csi_addr;
 } __attribute__((aligned(8)));
-#endif
 
 struct ckpt_hdr_sigpending {
 	struct ckpt_hdr h;
diff --git a/kernel/signal.c b/kernel/signal.c
index e4ca9a6..07647d7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2915,42 +2915,42 @@ static const struct ckpt_obj_ops ckpt_obj_sighand_ops = {
 
 static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
 {
-	si->signo = info->si_signo;
-	si->_errno = info->si_errno;
-	si->code = info->si_code;
+	si->csi_signo = info->si_signo;
+	si->csi_errno = info->si_errno;
+	si->csi_code = info->si_code;
 
 	/* TODO: convert info->si_uid to uid_objref */
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_TIMER:
-		si->pid = info->si_tid;
-		si->uid = info->si_overrun;
-		si->sigval_int = info->si_int;
-		si->utime = info->si_sys_private;
+		si->csi_pid = info->si_tid;
+		si->csi_uid = info->si_overrun;
+		si->csi_int = info->si_int;
+		si->csi_utime = info->si_sys_private;
 		break;
 	case __SI_POLL:
-		si->pid = info->si_band;
-		si->sigval_int = info->si_fd;
+		si->csi_pid = info->si_band;
+		si->csi_int = info->si_fd;
 		break;
 	case __SI_FAULT:
-		si->sigval_ptr = (unsigned long) info->si_addr;
+		si->csi_ptr = (unsigned long) info->si_addr;
 #ifdef __ARCH_SI_TRAPNO
-		si->sigval_int = info->si_trapno;
+		si->csi_int = info->si_trapno;
 #endif
 		break;
 	case __SI_CHLD:
-		si->pid = info->si_pid;
-		si->uid = info->si_uid;
-		si->sigval_int = info->si_status;
-		si->stime = info->si_stime;
-		si->utime = info->si_utime;
+		si->csi_pid = info->si_pid;
+		si->csi_uid = info->si_uid;
+		si->csi_int = info->si_status;
+		si->csi_stime = info->si_stime;
+		si->csi_utime = info->si_utime;
 		break;
 	case __SI_KILL:
 	case __SI_RT:
 	case __SI_MESGQ:
-		si->pid = info->si_pid;
-		si->uid = info->si_uid;
-		si->sigval_ptr = (unsigned long) info->si_ptr;
+		si->csi_pid = info->si_pid;
+		si->csi_uid = info->si_uid;
+		si->csi_ptr = (unsigned long) info->si_ptr;
 		break;
 	default:
 		BUG();
@@ -2959,47 +2959,47 @@ static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
 
 static int load_siginfo(siginfo_t *info, struct ckpt_siginfo *si)
 {
-	if (!valid_signal(si->signo))
+	if (!valid_signal(si->csi_signo))
 		return -EINVAL;
-	if (!ckpt_validate_errno(si->_errno))
+	if (!ckpt_validate_errno(si->csi_errno))
 		return -EINVAL;
 
-	info->si_signo = si->signo;
-	info->si_errno = si->_errno;
-	info->si_code = si->code;
+	info->si_signo = si->csi_signo;
+	info->si_errno = si->csi_errno;
+	info->si_code = si->csi_code;
 
 	/* TODO: validate remaining signal fields */
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_TIMER:
-		info->si_tid = si->pid;
-		info->si_overrun = si->uid;
-		info->si_int = si->sigval_int;
-		info->si_sys_private = si->utime;
+		info->si_tid = si->csi_pid;
+		info->si_overrun = si->csi_uid;
+		info->si_int = si->csi_int;
+		info->si_sys_private = si->csi_utime;
 		break;
 	case __SI_POLL:
-		info->si_band = si->pid;
-		info->si_fd = si->sigval_int;
+		info->si_band = si->csi_pid;
+		info->si_fd = si->csi_int;
 		break;
 	case __SI_FAULT:
-		info->si_addr = (void __user *) (unsigned long) si->sigval_ptr;
+		info->si_addr = (void __user *) (unsigned long) si->csi_ptr;
 #ifdef __ARCH_SI_TRAPNO
-		info->si_trapno = si->sigval_int;
+		info->si_trapno = si->csi_int;
 #endif
 		break;
 	case __SI_CHLD:
-		info->si_pid = si->pid;
-		info->si_uid = si->uid;
-		info->si_status = si->sigval_int;
-		info->si_stime = si->stime;
-		info->si_utime = si->utime;
+		info->si_pid = si->csi_pid;
+		info->si_uid = si->csi_uid;
+		info->si_status = si->csi_int;
+		info->si_stime = si->csi_stime;
+		info->si_utime = si->csi_utime;
 		break;
 	case __SI_KILL:
 	case __SI_RT:
 	case __SI_MESGQ:
-		info->si_pid = si->pid;
-		info->si_uid = si->uid;
-		info->si_ptr = (void __user *) (unsigned long) si->sigval_ptr;
+		info->si_pid = si->csi_pid;
+		info->si_uid = si->csi_uid;
+		info->si_ptr = (void __user *) (unsigned long) si->csi_ptr;
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.1.1

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

* [PATCH 2/8] checkpoint: add const qualifiers to siginfo handlers
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
  2010-07-13 15:44   ` [PATCH 1/8] checkpoint: model ckpt_siginfo on signalfd_siginfo Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
       [not found]     ` <1279035864-10533-3-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
  2010-07-13 15:44   ` [PATCH 3/8] checkpoint: clean up pending posix timer signal handling Nathan Lynch
                     ` (7 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

fill_siginfo and load_siginfo both have parameters that can be const.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 07647d7..13ab68e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2913,7 +2913,7 @@ static const struct ckpt_obj_ops ckpt_obj_sighand_ops = {
  * signal checkpoint/restart
  */
 
-static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
+static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 {
 	si->csi_signo = info->si_signo;
 	si->csi_errno = info->si_errno;
@@ -2957,7 +2957,7 @@ static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
 	}
 }
 
-static int load_siginfo(siginfo_t *info, struct ckpt_siginfo *si)
+static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 {
 	if (!valid_signal(si->csi_signo))
 		return -EINVAL;
-- 
1.7.1.1

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

* [PATCH 3/8] checkpoint: clean up pending posix timer signal handling
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
  2010-07-13 15:44   ` [PATCH 1/8] checkpoint: model ckpt_siginfo on signalfd_siginfo Nathan Lynch
  2010-07-13 15:44   ` [PATCH 2/8] checkpoint: add const qualifiers to siginfo handlers Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 15:44   ` [PATCH 4/8] checkpoint: clean up __SI_POLL siginfo save/restore Nathan Lynch
                     ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Add a field to ckpt_siginfo corresponding to the si_sys_private
siginfo field.  Stop overloading unrelated fields and use the
appropriate members for timer id and overrun.

si_int is a member of union sigval, whose other member is a pointer;
saving and restoring si_int will work for 32- but not 64-bit
architectures.  Save and restore the si_ptr field instead, which will
cover the whole union.

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

diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index b46d586..16b1723 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -936,6 +936,7 @@ struct ckpt_siginfo {
 	__u64 csi_utime;
 	__u64 csi_stime;
 	__u64 csi_addr;
+	__s32 csi_sys_private; /* POSIX.1b timers */
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_sigpending {
diff --git a/kernel/signal.c b/kernel/signal.c
index 13ab68e..e78a08e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2923,10 +2923,10 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_TIMER:
-		si->csi_pid = info->si_tid;
-		si->csi_uid = info->si_overrun;
-		si->csi_int = info->si_int;
-		si->csi_utime = info->si_sys_private;
+		si->csi_tid = info->si_tid;
+		si->csi_overrun = info->si_overrun;
+		si->csi_ptr = (unsigned long)info->si_ptr;
+		si->csi_sys_private = info->si_sys_private;
 		break;
 	case __SI_POLL:
 		si->csi_pid = info->si_band;
@@ -2972,10 +2972,10 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_TIMER:
-		info->si_tid = si->csi_pid;
-		info->si_overrun = si->csi_uid;
-		info->si_int = si->csi_int;
-		info->si_sys_private = si->csi_utime;
+		info->si_tid = si->csi_tid;
+		info->si_overrun = si->csi_overrun;
+		info->si_ptr = (void __user *)(unsigned long)si->csi_ptr;
+		info->si_sys_private = si->csi_sys_private;
 		break;
 	case __SI_POLL:
 		info->si_band = si->csi_pid;
-- 
1.7.1.1

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

* [PATCH 4/8] checkpoint: clean up __SI_POLL siginfo save/restore
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 3/8] checkpoint: clean up pending posix timer signal handling Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 15:44   ` [PATCH 5/8] checkpoint: clean up __SI_FAULT " Nathan Lynch
                     ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

ckpt_siginfo now has fields for band and fd; use them for the
__SI_POLL case.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index e78a08e..df9b7be 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2929,8 +2929,8 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 		si->csi_sys_private = info->si_sys_private;
 		break;
 	case __SI_POLL:
-		si->csi_pid = info->si_band;
-		si->csi_int = info->si_fd;
+		si->csi_band = info->si_band;
+		si->csi_fd = info->si_fd;
 		break;
 	case __SI_FAULT:
 		si->csi_ptr = (unsigned long) info->si_addr;
@@ -2978,8 +2978,8 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 		info->si_sys_private = si->csi_sys_private;
 		break;
 	case __SI_POLL:
-		info->si_band = si->csi_pid;
-		info->si_fd = si->csi_int;
+		info->si_band = si->csi_band;
+		info->si_fd = si->csi_fd;
 		break;
 	case __SI_FAULT:
 		info->si_addr = (void __user *) (unsigned long) si->csi_ptr;
-- 
1.7.1.1

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

* [PATCH 5/8] checkpoint: clean up __SI_FAULT siginfo save/restore
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 4/8] checkpoint: clean up __SI_POLL siginfo save/restore Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 15:44   ` [PATCH 6/8] checkpoint: clean up __SI_CHLD " Nathan Lynch
                     ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

ckpt_siginfo now has addr and trapno fields, so use them for the
__SI_FAULT case.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index df9b7be..de85c35 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2933,9 +2933,9 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 		si->csi_fd = info->si_fd;
 		break;
 	case __SI_FAULT:
-		si->csi_ptr = (unsigned long) info->si_addr;
+		si->csi_addr = (unsigned long)info->si_addr;
 #ifdef __ARCH_SI_TRAPNO
-		si->csi_int = info->si_trapno;
+		si->csi_trapno = info->si_trapno;
 #endif
 		break;
 	case __SI_CHLD:
@@ -2982,9 +2982,9 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 		info->si_fd = si->csi_fd;
 		break;
 	case __SI_FAULT:
-		info->si_addr = (void __user *) (unsigned long) si->csi_ptr;
+		info->si_addr = (void __user *)(unsigned long)si->csi_addr;
 #ifdef __ARCH_SI_TRAPNO
-		info->si_trapno = si->csi_int;
+		info->si_trapno = si->csi_trapno;
 #endif
 		break;
 	case __SI_CHLD:
-- 
1.7.1.1

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

* [PATCH 6/8] checkpoint: clean up __SI_CHLD siginfo save/restore
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (4 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 5/8] checkpoint: clean up __SI_FAULT " Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 15:44   ` [PATCH 7/8] checkpoint: __SI_KILL does not use si_ptr Nathan Lynch
                     ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

ckpt_siginfo now has a field for exit status; use it.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index de85c35..fccf9b6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2941,7 +2941,7 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 	case __SI_CHLD:
 		si->csi_pid = info->si_pid;
 		si->csi_uid = info->si_uid;
-		si->csi_int = info->si_status;
+		si->csi_status = info->si_status;
 		si->csi_stime = info->si_stime;
 		si->csi_utime = info->si_utime;
 		break;
@@ -2990,7 +2990,7 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 	case __SI_CHLD:
 		info->si_pid = si->csi_pid;
 		info->si_uid = si->csi_uid;
-		info->si_status = si->csi_int;
+		info->si_status = si->csi_status;
 		info->si_stime = si->csi_stime;
 		info->si_utime = si->csi_utime;
 		break;
-- 
1.7.1.1

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

* [PATCH 7/8] checkpoint: __SI_KILL does not use si_ptr
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (5 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 6/8] checkpoint: clean up __SI_CHLD " Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 15:44   ` [PATCH 8/8] checkpoint: handle siginfo->si_code < 0 Nathan Lynch
                     ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Handle __SI_KILL in its own case during siginfo save/restore; grouping
it with __SI_RT and __SI_MESGQ doesn't make sense as si_ptr use is not
implied.  See copy_siginfo_to_user, signalfd_copyinfo.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index fccf9b6..85757e1 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2922,6 +2922,10 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 	/* TODO: convert info->si_uid to uid_objref */
 
 	switch (info->si_code & __SI_MASK) {
+	case __SI_KILL:
+		si->csi_pid = info->si_pid;
+		si->csi_uid = info->si_uid;
+		break;
 	case __SI_TIMER:
 		si->csi_tid = info->si_tid;
 		si->csi_overrun = info->si_overrun;
@@ -2945,12 +2949,11 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 		si->csi_stime = info->si_stime;
 		si->csi_utime = info->si_utime;
 		break;
-	case __SI_KILL:
 	case __SI_RT:
 	case __SI_MESGQ:
 		si->csi_pid = info->si_pid;
 		si->csi_uid = info->si_uid;
-		si->csi_ptr = (unsigned long) info->si_ptr;
+		si->csi_ptr = (unsigned long)info->si_ptr;
 		break;
 	default:
 		BUG();
@@ -2971,6 +2974,10 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 	/* TODO: validate remaining signal fields */
 
 	switch (info->si_code & __SI_MASK) {
+	case __SI_KILL:
+		info->si_pid = si->csi_pid;
+		info->si_uid = si->csi_uid;
+		break;
 	case __SI_TIMER:
 		info->si_tid = si->csi_tid;
 		info->si_overrun = si->csi_overrun;
@@ -2994,12 +3001,11 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 		info->si_stime = si->csi_stime;
 		info->si_utime = si->csi_utime;
 		break;
-	case __SI_KILL:
 	case __SI_RT:
 	case __SI_MESGQ:
 		info->si_pid = si->csi_pid;
 		info->si_uid = si->csi_uid;
-		info->si_ptr = (void __user *) (unsigned long) si->csi_ptr;
+		info->si_ptr = (void __user *)(unsigned long)si->csi_ptr;
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.1.1

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

* [PATCH 8/8] checkpoint: handle siginfo->si_code < 0
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (6 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 7/8] checkpoint: __SI_KILL does not use si_ptr Nathan Lynch
@ 2010-07-13 15:44   ` Nathan Lynch
  2010-07-13 22:22   ` [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments Nathan Lynch
  2010-07-20  3:00   ` [PATCH 0/8] checkpoint: improve handling of pending signals Oren Laadan
  9 siblings, 0 replies; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 15:44 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

If we checkpoint a task that has a pending signal with si_code < 0
(easily caused by raising a blocked signal) we hit the BUG() in
fill_siginfo.  In this case, we should checkpoint and restore si_pid,
si_uid, and the members of the si_val union.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 kernel/signal.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 85757e1..cc7aee9 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2920,6 +2920,25 @@ static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
 	si->csi_code = info->si_code;
 
 	/* TODO: convert info->si_uid to uid_objref */
+	if (info->si_code < 0) {
+		switch(info->si_code) {
+		case SI_QUEUE:
+		case SI_TIMER:
+		case SI_MESGQ:
+		case SI_ASYNCIO:
+		case SI_SIGIO:
+		case SI_TKILL:
+		case SI_DETHREAD:
+			si->csi_pid = info->si_pid;
+			si->csi_uid = info->si_uid;
+			si->csi_ptr = (unsigned long)info->si_ptr;
+			si->csi_int = info->si_int;
+			break;
+		default:
+			BUG();
+		}
+		return;
+	}
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_KILL:
@@ -2972,6 +2991,27 @@ static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
 	info->si_code = si->csi_code;
 
 	/* TODO: validate remaining signal fields */
+	if (info->si_code < 0) {
+		switch(info->si_code) {
+		case SI_QUEUE:
+		case SI_TIMER:
+		case SI_MESGQ:
+		case SI_ASYNCIO:
+		case SI_SIGIO:
+		case SI_TKILL:
+		case SI_DETHREAD:
+			info->si_pid = si->csi_pid;
+			info->si_uid = si->csi_uid;
+			info->si_ptr = (void __user *)
+				(unsigned long)si->csi_ptr;
+			info->si_int = si->csi_int;
+			break;
+		default:
+			return -EINVAL;
+			break;
+		}
+		return 0;
+	}
 
 	switch (info->si_code & __SI_MASK) {
 	case __SI_KILL:
-- 
1.7.1.1

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

* [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (7 preceding siblings ...)
  2010-07-13 15:44   ` [PATCH 8/8] checkpoint: handle siginfo->si_code < 0 Nathan Lynch
@ 2010-07-13 22:22   ` Nathan Lynch
  2010-07-13 23:36     ` Matt Helsley
  2010-07-20  3:00   ` [PATCH 0/8] checkpoint: improve handling of pending signals Oren Laadan
  9 siblings, 1 reply; 14+ messages in thread
From: Nathan Lynch @ 2010-07-13 22:22 UTC (permalink / raw)
  To: orenl-eQaUEPhvms7ENvBUuze7eA
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

A testcase that posts several realtime signals via sigqueue before C/R
uncovered this.  Without this change only the first queued signal is
delivered after restart.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---

[only noticed this after posting the series, oops]

 kernel/signal.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index cc7aee9..ba8a623 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3277,7 +3277,7 @@ static int restore_sigpending(struct ckpt_ctx *ctx, struct sigpending *pending)
 		}
 
 		q->flags &= ~SIGQUEUE_PREALLOC;
-		list_add_tail(&pending->list, &q->list);
+		list_add_tail(&q->list, &pending->list);
 	}
 
 	if (ret < 0)
-- 
1.7.1.1

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

* Re: [PATCH 2/8] checkpoint: add const qualifiers to siginfo handlers
       [not found]     ` <1279035864-10533-3-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
@ 2010-07-13 23:29       ` Matt Helsley
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Helsley @ 2010-07-13 23:29 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Tue, Jul 13, 2010 at 10:44:18AM -0500, Nathan Lynch wrote:
> fill_siginfo and load_siginfo both have parameters that can be const.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Matt Helsley <matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> ---
>  kernel/signal.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 07647d7..13ab68e 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2913,7 +2913,7 @@ static const struct ckpt_obj_ops ckpt_obj_sighand_ops = {
>   * signal checkpoint/restart
>   */
> 
> -static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
> +static void fill_siginfo(struct ckpt_siginfo *si, const siginfo_t *info)
>  {
>  	si->csi_signo = info->si_signo;
>  	si->csi_errno = info->si_errno;
> @@ -2957,7 +2957,7 @@ static void fill_siginfo(struct ckpt_siginfo *si, siginfo_t *info)
>  	}
>  }
> 
> -static int load_siginfo(siginfo_t *info, struct ckpt_siginfo *si)
> +static int load_siginfo(siginfo_t *info, const struct ckpt_siginfo *si)
>  {
>  	if (!valid_signal(si->csi_signo))
>  		return -EINVAL;
> -- 
> 1.7.1.1
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

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

* Re: [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments
  2010-07-13 22:22   ` [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments Nathan Lynch
@ 2010-07-13 23:36     ` Matt Helsley
  0 siblings, 0 replies; 14+ messages in thread
From: Matt Helsley @ 2010-07-13 23:36 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

On Tue, Jul 13, 2010 at 05:22:57PM -0500, Nathan Lynch wrote:
> A testcase that posts several realtime signals via sigqueue before C/R
> uncovered this.  Without this change only the first queued signal is
> delivered after restart.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Nice catch!

Reviewed-by: Matt Helsley <matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> ---
> 
> [only noticed this after posting the series, oops]
> 
>  kernel/signal.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index cc7aee9..ba8a623 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3277,7 +3277,7 @@ static int restore_sigpending(struct ckpt_ctx *ctx, struct sigpending *pending)
>  		}
> 
>  		q->flags &= ~SIGQUEUE_PREALLOC;
> -		list_add_tail(&pending->list, &q->list);
> +		list_add_tail(&q->list, &pending->list);
>  	}
> 
>  	if (ret < 0)
> -- 
> 1.7.1.1
> 
> 
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

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

* Re: [PATCH 1/8] checkpoint: model ckpt_siginfo on signalfd_siginfo
       [not found]     ` <1279035864-10533-2-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
@ 2010-07-16  0:55       ` Sukadev Bhattiprolu
  0 siblings, 0 replies; 14+ messages in thread
From: Sukadev Bhattiprolu @ 2010-07-16  0:55 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

Nathan Lynch [ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org] wrote:
| ckpt_siginfo doesn't cover all of the possible siginfo fields; the
| current code overloads some fields (such as si_uid for si_overrun),
| which is confusing.  Signed-ness on some fields is not correct either.
| 
| signalfd_siginfo is a straightforward format for representing siginfo,
| so copy it and update the signal checkpoint code to use the new
| fields.  Although the ckpt_siginfo struct grows, there should be no
| behavioral changes.

| 
| Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Switching to the signalfd_siginfo style makes it more easier to
maintain.  So for all patches in this set:

Reviewed-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

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

* Re: [PATCH 0/8] checkpoint: improve handling of pending signals
       [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
                     ` (8 preceding siblings ...)
  2010-07-13 22:22   ` [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments Nathan Lynch
@ 2010-07-20  3:00   ` Oren Laadan
  9 siblings, 0 replies; 14+ messages in thread
From: Oren Laadan @ 2010-07-20  3:00 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA


Thanks Nathan - acking and pulling entire series including
the extra one to v22-dev.

Oren.

On 07/13/2010 11:44 AM, Nathan Lynch wrote:
> Hi Oren,
> 
> The following series improves the support for checkpointing pending
> signal state and (I think) is exhaustive.  These have been tested on
> top of your ckpt-v22-dev branch on x86 and powerpc systems for the
> last couple of weeks.
> 
> Nathan Lynch (8):
>   checkpoint: model ckpt_siginfo on signalfd_siginfo
>   checkpoint: add const qualifiers to siginfo handlers
>   checkpoint: clean up pending posix timer signal handling
>   checkpoint: clean up __SI_POLL siginfo save/restore
>   checkpoint: clean up __SI_FAULT siginfo save/restore
>   checkpoint: clean up __SI_CHLD siginfo save/restore
>   checkpoint: __SI_KILL does not use si_ptr
>   checkpoint: handle siginfo->si_code < 0
> 
>  include/linux/checkpoint_hdr.h |   29 +++++----
>  kernel/signal.c                |  134 +++++++++++++++++++++++++++-------------
>  2 files changed, 107 insertions(+), 56 deletions(-)
> 
> 

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

end of thread, other threads:[~2010-07-20  3:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-13 15:44 [PATCH 0/8] checkpoint: improve handling of pending signals Nathan Lynch
     [not found] ` <1279035864-10533-1-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
2010-07-13 15:44   ` [PATCH 1/8] checkpoint: model ckpt_siginfo on signalfd_siginfo Nathan Lynch
     [not found]     ` <1279035864-10533-2-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
2010-07-16  0:55       ` Sukadev Bhattiprolu
2010-07-13 15:44   ` [PATCH 2/8] checkpoint: add const qualifiers to siginfo handlers Nathan Lynch
     [not found]     ` <1279035864-10533-3-git-send-email-ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
2010-07-13 23:29       ` Matt Helsley
2010-07-13 15:44   ` [PATCH 3/8] checkpoint: clean up pending posix timer signal handling Nathan Lynch
2010-07-13 15:44   ` [PATCH 4/8] checkpoint: clean up __SI_POLL siginfo save/restore Nathan Lynch
2010-07-13 15:44   ` [PATCH 5/8] checkpoint: clean up __SI_FAULT " Nathan Lynch
2010-07-13 15:44   ` [PATCH 6/8] checkpoint: clean up __SI_CHLD " Nathan Lynch
2010-07-13 15:44   ` [PATCH 7/8] checkpoint: __SI_KILL does not use si_ptr Nathan Lynch
2010-07-13 15:44   ` [PATCH 8/8] checkpoint: handle siginfo->si_code < 0 Nathan Lynch
2010-07-13 22:22   ` [PATCH 9/8] restore_sigpending: fix reversed list_add_tail arguments Nathan Lynch
2010-07-13 23:36     ` Matt Helsley
2010-07-20  3:00   ` [PATCH 0/8] checkpoint: improve handling of pending signals Oren Laadan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox