Quoting Serge E. Hallyn (serue@xxxxxxxxxx): > Well, perhaps I've been making this more complicated than it > needs to be. > > If we get frozen+checkpointed, then no matter whether we were > frozen with a real pending signal or not, we won't handle the > signal during restart, so we can treat it as though signr==0. > > So, in that case, the only thing we need to change at end of > sys_restart is to handle the case: > > /* Restart a different system call. */ > if (retval == -ERESTART_RESTARTBLOCK > && regs->psw.addr == continue_addr) { > regs->gprs[2] = __NR_restart_syscall; > set_thread_flag(TIF_RESTART_SVC); > } > > Now that's of course a problem bc we don't know continue_addr > when we're in sys_restart(). So before we go into get_signal_to_deliver(), > we should set a new TIF flag which represents the fact that we are > inside do_signal with those conditions. > > Then at end of restore_thread(), if that flag is set, we do the > > regs->gprs[2] = __NR_restart_syscall; > set_thread_flag(TIF_RESTART_SVC); > > (which presumably goes into a helper) > > If there was a pending signal which we were intending to handle > when checkpointed, then that will simply be delivered after we > exit sys_restart. That is no different from the case where we > got another signal delivered while a slow sighandler was executing. > > I'll try implementing that idea. > > -serge Like so: >From b50eb90da03e82ae6e7a2f1a8362e93c18d52074 Mon Sep 17 00:00:00 2001 From: Serge E. Hallyn <serue@xxxxxxxxxx> Date: Thu, 11 Feb 2010 14:05:16 -0500 Subject: [PATCH 1/1] set TIF_RESTART_SVC when needed after sys_restart Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx> --- arch/s390/include/asm/checkpoint_hdr.h | 5 +++ arch/s390/include/asm/thread_info.h | 2 + arch/s390/kernel/checkpoint.c | 52 +++++++++++++++++++++++++++++++- arch/s390/kernel/signal.c | 16 ++++++++++ 4 files changed, 74 insertions(+), 1 deletions(-) diff --git a/arch/s390/include/asm/checkpoint_hdr.h b/arch/s390/include/asm/checkpoint_hdr.h index a8c2a3d..fd8be2a 100644 --- a/arch/s390/include/asm/checkpoint_hdr.h +++ b/arch/s390/include/asm/checkpoint_hdr.h @@ -76,6 +76,11 @@ struct ckpt_hdr_cpu { __u8 instruction_fetch; }; +struct ckpt_hdr_thread { + struct ckpt_hdr h; + __u64 thread_info_flags; +}; + struct ckpt_hdr_mm_context { struct ckpt_hdr h; unsigned long vdso_base; diff --git a/arch/s390/include/asm/thread_info.h b/arch/s390/include/asm/thread_info.h index 66069e7..61e84e5 100644 --- a/arch/s390/include/asm/thread_info.h +++ b/arch/s390/include/asm/thread_info.h @@ -99,6 +99,7 @@ static inline struct thread_info *current_thread_info(void) #define TIF_MEMDIE 18 #define TIF_RESTORE_SIGMASK 19 /* restore signal mask in do_signal() */ #define TIF_FREEZE 20 /* thread is freezing for suspend */ +#define TIF_SIG_RESTARTBLOCK 23 /* restart must set TIF_RESTART_SVC */ #define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME) #define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) @@ -114,6 +115,7 @@ static inline struct thread_info *current_thread_info(void) #define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG) #define _TIF_31BIT (1<<TIF_31BIT) #define _TIF_FREEZE (1<<TIF_FREEZE) +#define _TIF_SIG_RESTARTBLOCK (1<<TIF_SIG_RESTARTBLOCK) #endif /* __KERNEL__ */ diff --git a/arch/s390/kernel/checkpoint.c b/arch/s390/kernel/checkpoint.c index 60ba04d..cc7f341 100644 --- a/arch/s390/kernel/checkpoint.c +++ b/arch/s390/kernel/checkpoint.c @@ -12,6 +12,7 @@ #include <asm/system.h> #include <asm/pgtable.h> #include <asm/elf.h> +#include <asm/unistd.h> #include <linux/checkpoint.h> #include <linux/checkpoint_hdr.h> @@ -65,6 +66,18 @@ static void s390_copy_regs(int op, struct ckpt_hdr_cpu *h, BUG_ON(h->gprs[2] < 0); h->gprs[2] = 0; } + + /* + * if a checkpoint was taken while interrupted from a restartable + * syscall, then treat this as though signr==0 (since we did not + * handle the signal) and finish the last part of do_signal + */ + if (op == CKPT_RST && test_thread_flag(TIF_SIG_RESTARTBLOCK)) { + regs->gprs[2] = __NR_restart_syscall; + set_thread_flag(TIF_RESTART_SVC); + clear_thread_flag(TIF_SIG_RESTARTBLOCK); + } + CKPT_COPY_ARRAY(op, h->fprs, thr->fp_regs.fprs, NUM_FPRS); CKPT_COPY_ARRAY(op, h->acrs, thr->acrs, NUM_ACRS); CKPT_COPY_ARRAY(op, h->per_control_regs, @@ -83,12 +96,24 @@ static void s390_mm(int op, struct ckpt_hdr_mm_context *h, int checkpoint_thread(struct ckpt_ctx *ctx, struct task_struct *t) { + struct ckpt_hdr_thread *h; + int ret; + /* we will eventually support this, as we do on x86-64 */ if (test_tsk_thread_flag(t, TIF_31BIT)) { ckpt_err(ctx, -EINVAL, "checkpoint of 31-bit task\n"); return -EINVAL; } - return 0; + + h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_THREAD); + if (!h) + return -ENOMEM; + + h->thread_info_flags = task_thread_info(t)->flags; + ret = ckpt_write_obj(ctx, &h->h); + ckpt_hdr_put(ctx, h); + + return ret; } /* dump the cpu state and registers of a given task */ @@ -148,11 +173,36 @@ int checkpoint_mm_context(struct ckpt_ctx *ctx, struct mm_struct *mm) int restore_thread(struct ckpt_ctx *ctx) { + struct ckpt_hdr_thread *h; + /* a 31-bit task cannot call sys_restart right now */ if (test_thread_flag(TIF_31BIT)) { ckpt_err(ctx, -EINVAL, "restart from 31-bit task\n"); return -EINVAL; } + + h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_THREAD); + if (IS_ERR(h)) + return PTR_ERR(h); + + /* + * If we were checkpointed while do_signal() interrupted a + * syscall with restart blocks, then we have some cleanup to + * do at end of restart, in order to finish our pretense of + * having handled signr==0. (See last part of do_signal). + * + * We can't set gprs[2] here bc we haven't copied registers + * yet, that happens later in restore_cpu(). So re-set the + * TIF_SIG_RESTARTBLOCK thread flag so we can detect it + * at restore_cpu()->s390_copy_regs() and do the right thing. + */ + if (h->thread_info_flags & _TIF_SIG_RESTARTBLOCK) + set_thread_flag(TIF_SIG_RESTARTBLOCK); + + /* will have to handle TIF_RESTORE_SIGMASK as well */ + + ckpt_hdr_put(ctx, h); + return 0; } diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c index 1675c48..9b6ca21 100644 --- a/arch/s390/kernel/signal.c +++ b/arch/s390/kernel/signal.c @@ -459,6 +459,16 @@ void do_signal(struct pt_regs *regs) break; case -ERESTART_RESTARTBLOCK: regs->gprs[2] = -EINTR; + /* + * This condition is the only one which requires + * special care after handling a signr==0. So if + * we get frozen and checkpointed at the + * get_signal_to_deliver() below, then we need + * to convey this condition to sys_restart() so it + * can set the restored thread up to run the restart + * block. + */ + set_thread_flag(TIF_SIG_RESTARTBLOCK); } regs->svcnr = 0; /* Don't deal with this again. */ } @@ -467,6 +477,12 @@ void do_signal(struct pt_regs *regs) the debugger may change all our registers ... */ signr = get_signal_to_deliver(&info, &ka, regs, NULL); + /* + * we won't get frozen past this so clear the thread flag hinting + * to sys_restart that TIF_RESTART_SVC must be set. + */ + clear_thread_flag(TIF_SIG_RESTARTBLOCK); + /* Depending on the signal settings we may need to revert the decision to restart the system call. */ if (signr > 0 && regs->psw.addr == restart_addr) { -- 1.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html