checkpoint: handle saved_sigmask

RFC PATCH In review Serge E. Hallyn 3 participants 4 messages Feb 2, 2010 – Feb 18, 2010 3 messages not in the archive

Subsystems (3)

x86 PowerPC s390

lore

Share & tools

lore

Serge E. Hallyn

author

2010-02-02 18:07 UTC #0 lore raw

RFC PATCH 1/1 checkpoint: handle saved_sigmask

Each arch seems to have its own way of specifying that current->saved_sigmask should contains the valid blocked sigmask. So provide an arch-specific hook for that test, and, if true, then store saved_sigmask in place of blocked in the checkpoint image. None of the architectures currently save these flags, so we don't need to do anything special to unset them after restart.

If right after sys_restart the a signal needs to be handled (which was sent during restart) then it will just do the right thing (save blocked back to saved_sigmask and set TIF_RESTORE_SIGMASK or whatever).

Thoughts?

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 arch/powerpc/kernel/signal.c  |    9 +++++++++
 arch/s390/kernel/checkpoint.c |    2 --
 arch/s390/kernel/signal.c     |    7 +++++++
 arch/x86/kernel/signal.c      |    6 ++++++
 checkpoint/signal.c           |    5 ++++-
 include/linux/signal.h        |    6 ++++++
 6 files changed, 32 insertions(+), 3 deletions(-)

arch/powerpc/kernel/signal.c +9 -0

index 00b5078..4aa0128 100644
@@ -188,6 +188,15 @@ static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 	return ret;
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	struct thread_info *ti = task_thread_info(task);
+	if (ti->local_flags & _TLF_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
+
 void do_signal(struct pt_regs *regs, unsigned long thread_info_flags)
 {
 	if (thread_info_flags & _TIF_SIGPENDING)

arch/s390/kernel/checkpoint.c +0 -2

index 092fd87..048e820 100644
@@ -200,8 +200,6 @@ int restore_thread(struct ckpt_ctx *ctx)
 		set_thread_flag(TIF_RESTARTBLOCK);
 	}
 
-	/* need to do something with TIF_RESTORE_SIGMASK ? */
-
 	ckpt_hdr_put(ctx, h);
 	return 0;
 }

arch/s390/kernel/signal.c +7 -0

index 503fd09..0654f12 100644
@@ -538,6 +538,13 @@ void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	if (test_tsk_thread_flag(task, TIF_RESTORE_SIGMASK))
+		return 1;
+	return 0;
+}
+
 void do_notify_resume(struct pt_regs *regs)
 {
 	clear_thread_flag(TIF_NOTIFY_RESUME);

arch/x86/kernel/signal.c +6 -0

index 6a44a76..a6e8035 100644
@@ -849,6 +849,12 @@ static void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task) {
+	if (task_thread_info(t)->status & TS_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
 /*
  * notification of userspace execution resumption
  * - triggered by the TIF_WORK_MASK flags

checkpoint/signal.c +4 -1

index 609e924..ad95e0c 100644
@@ -682,7 +682,10 @@ int checkpoint_task_signal(struct ckpt_ctx *ctx, struct task_struct *t)
 	if (!h)
 		return -ENOMEM;
 
-	fill_sigset(&h->blocked, &t->blocked);
+	if (task_restore_sigmask(t))
+		fill_sigset(&h->blocked, &t->saved_sigmask);
+	else
+		fill_sigset(&h->blocked, &t->blocked);
 
 	ret = ckpt_write_obj(ctx, &h->h);
 	ckpt_hdr_put(ctx, h);

include/linux/signal.h +6 -0

index ab9272c..aa18432 100644
@@ -376,6 +376,12 @@ int unhandled_signal(struct task_struct *tsk, int sig);
 
 void signals_init(void);
 
+/*
+ * arch-specific query, used at checkpoint: should saved_sigmask be used
+ * in place of blocked
+ */
+int task_restore_sigmask(struct task_struct *task);
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SIGNAL_H */

Signature · 1 line

1.6.1

A message that is not in the archive (sent off-list or not yet received): 20100202180718.GA28249-r/[email protected]

Serge E. Hallyn

author

2010-02-18 16:08 UTC #2 lore raw

I thought there had been followup discussion on this, but I don't see it in my mbox. (maybe it was on irc?)

Did we decide to punt on this until v20?

-serge

Quoted 133 lines · Serge E. Hallyn

Thoughts?

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 arch/powerpc/kernel/signal.c  |    9 +++++++++
 arch/s390/kernel/checkpoint.c |    2 --
 arch/s390/kernel/signal.c     |    7 +++++++
 arch/x86/kernel/signal.c      |    6 ++++++
 checkpoint/signal.c           |    5 ++++-
 include/linux/signal.h        |    6 ++++++
 6 files changed, 32 insertions(+), 3 deletions(-)

arch/powerpc/kernel/signal.c +9 -0

index 00b5078..4aa0128 100644
@@ -188,6 +188,15 @@ static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 	return ret;
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	struct thread_info *ti = task_thread_info(task);
+	if (ti->local_flags & _TLF_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
+
 void do_signal(struct pt_regs *regs, unsigned long thread_info_flags)
 {
 	if (thread_info_flags & _TIF_SIGPENDING)

arch/s390/kernel/checkpoint.c +0 -2

index 092fd87..048e820 100644
@@ -200,8 +200,6 @@ int restore_thread(struct ckpt_ctx *ctx)
 		set_thread_flag(TIF_RESTARTBLOCK);
 	}
 
-	/* need to do something with TIF_RESTORE_SIGMASK ? */
-
 	ckpt_hdr_put(ctx, h);
 	return 0;
 }

arch/s390/kernel/signal.c +7 -0

index 503fd09..0654f12 100644
@@ -538,6 +538,13 @@ void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	if (test_tsk_thread_flag(task, TIF_RESTORE_SIGMASK))
+		return 1;
+	return 0;
+}
+
 void do_notify_resume(struct pt_regs *regs)
 {
 	clear_thread_flag(TIF_NOTIFY_RESUME);

arch/x86/kernel/signal.c +6 -0

index 6a44a76..a6e8035 100644
@@ -849,6 +849,12 @@ static void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task) {
+	if (task_thread_info(t)->status & TS_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
 /*
  * notification of userspace execution resumption
  * - triggered by the TIF_WORK_MASK flags

checkpoint/signal.c +4 -1

index 609e924..ad95e0c 100644
@@ -682,7 +682,10 @@ int checkpoint_task_signal(struct ckpt_ctx *ctx, struct task_struct *t)
 	if (!h)
 		return -ENOMEM;
 
-	fill_sigset(&h->blocked, &t->blocked);
+	if (task_restore_sigmask(t))
+		fill_sigset(&h->blocked, &t->saved_sigmask);
+	else
+		fill_sigset(&h->blocked, &t->blocked);
 
 	ret = ckpt_write_obj(ctx, &h->h);
 	ckpt_hdr_put(ctx, h);

include/linux/signal.h +6 -0

index ab9272c..aa18432 100644
@@ -376,6 +376,12 @@ int unhandled_signal(struct task_struct *tsk, int sig);
 
 void signals_init(void);
 
+/*
+ * arch-specific query, used at checkpoint: should saved_sigmask be used
+ * in place of blocked
+ */
+int task_restore_sigmask(struct task_struct *task);
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SIGNAL_H */

Signature · 6 lines

1.6.1

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20100218160846.GA9141-r/[email protected]

Oren Laadan

2010-02-18 16:50 UTC #4 lore raw

Quoted 2 lines · Serge E. Hallyn

I thought there had been followup discussion on this, but I don't see it in my mbox. (maybe it was on irc?)

The patch seems correct.

The only comment is that naming the hook "task_restore_sigmask" confused me to think it is part of restart logic, which it isn't.

How about "task_saved_sigmask" instead ?

Quoted 1 line

Did we decide to punt on this until v20?

Probably better to add to v19 while we're at it. I'll do it right away unless you object to the name change.

Oren.

Quoted 138 lines

-serge

Quoted 133 lines · Serge E. Hallyn

Thoughts?

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 arch/powerpc/kernel/signal.c  |    9 +++++++++
 arch/s390/kernel/checkpoint.c |    2 --
 arch/s390/kernel/signal.c     |    7 +++++++
 arch/x86/kernel/signal.c      |    6 ++++++
 checkpoint/signal.c           |    5 ++++-
 include/linux/signal.h        |    6 ++++++
 6 files changed, 32 insertions(+), 3 deletions(-)

arch/powerpc/kernel/signal.c +9 -0

index 00b5078..4aa0128 100644
@@ -188,6 +188,15 @@ static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 	return ret;
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	struct thread_info *ti = task_thread_info(task);
+	if (ti->local_flags & _TLF_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
+
 void do_signal(struct pt_regs *regs, unsigned long thread_info_flags)
 {
 	if (thread_info_flags & _TIF_SIGPENDING)

arch/s390/kernel/checkpoint.c +0 -2

index 092fd87..048e820 100644
@@ -200,8 +200,6 @@ int restore_thread(struct ckpt_ctx *ctx)
 		set_thread_flag(TIF_RESTARTBLOCK);
 	}
 
-	/* need to do something with TIF_RESTORE_SIGMASK ? */
-
 	ckpt_hdr_put(ctx, h);
 	return 0;
 }

arch/s390/kernel/signal.c +7 -0

index 503fd09..0654f12 100644
@@ -538,6 +538,13 @@ void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task)
+{
+	if (test_tsk_thread_flag(task, TIF_RESTORE_SIGMASK))
+		return 1;
+	return 0;
+}
+
 void do_notify_resume(struct pt_regs *regs)
 {
 	clear_thread_flag(TIF_NOTIFY_RESUME);

arch/x86/kernel/signal.c +6 -0

index 6a44a76..a6e8035 100644
@@ -849,6 +849,12 @@ static void do_signal(struct pt_regs *regs)
 	}
 }
 
+int task_restore_sigmask(struct task_struct *task) {
+	if (task_thread_info(t)->status & TS_RESTORE_SIGMASK)
+		return 1;
+	return 0;
+}
+
 /*
  * notification of userspace execution resumption
  * - triggered by the TIF_WORK_MASK flags

checkpoint/signal.c +4 -1

index 609e924..ad95e0c 100644
@@ -682,7 +682,10 @@ int checkpoint_task_signal(struct ckpt_ctx *ctx, struct task_struct *t)
 	if (!h)
 		return -ENOMEM;
 
-	fill_sigset(&h->blocked, &t->blocked);
+	if (task_restore_sigmask(t))
+		fill_sigset(&h->blocked, &t->saved_sigmask);
+	else
+		fill_sigset(&h->blocked, &t->blocked);
 
 	ret = ckpt_write_obj(ctx, &h->h);
 	ckpt_hdr_put(ctx, h);

include/linux/signal.h +6 -0

index ab9272c..aa18432 100644
@@ -376,6 +376,12 @@ int unhandled_signal(struct task_struct *tsk, int sig);
 
 void signals_init(void);
 
+/*
+ * arch-specific query, used at checkpoint: should saved_sigmask be used
+ * in place of blocked
+ */
+int task_restore_sigmask(struct task_struct *task);
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SIGNAL_H */

Signature · 6 lines

1.6.1

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): [email protected]

Serge E. Hallyn

2010-02-18 16:56 UTC #6 lore raw

Quoted 11 lines · Oren Laadan

Quoted 2 lines · Serge E. Hallyn
I thought there had been followup discussion on this, but I don't see it in my mbox. (maybe it was on irc?)
The patch seems correct.
The only comment is that naming the hook "task_restore_sigmask" confused me to think it is part of restart logic, which it isn't.

Quoted 6 lines

How about "task_saved_sigmask" instead ?
Quoted 1 line
Did we decide to punt on this until v20?
Probably better to add to v19 while we're at it. I'll do it right away unless you object to the name change.

no objections, thanks

-serge