From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
Keir Fraser <keir@xen.org>, Jan Beulich <JBeulich@suse.com>,
Tim Deegan <tim@xen.org>
Subject: [PATCH] x86/schedule: Remove noreturn from schedule_tail() function pointer
Date: Fri, 7 Mar 2014 11:05:08 +0000 [thread overview]
Message-ID: <1394190308-2465-1-git-send-email-andrew.cooper3@citrix.com> (raw)
XenServer has recently had a support case where this bugframe in
context_switch() was hit, presumably from a corrupt function pointer as the
vcpu pointer was fine.
On balance, it is better to leave the bugframe around for peace of mind in
exceptional circumstances, than to use the optimisations provided by noreturn.
At any meaningful levels of optimisation, the noreturn causes the bugframe to
be optimised out, meaning that any exceptional returns fall into unlikely
branches, which will result in very weird behaviour.
The unreachable() in BUG() does the useful part of noreturn for us, allowing
the compiler not to mess about restoring stack frames etc, but causes a ud2
instruction to be present.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
xen/include/asm-x86/current.h | 10 +++++++++-
xen/include/asm-x86/domain.h | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 4d1f20e..2081015 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -67,7 +67,15 @@ static inline struct cpu_info *get_cpu_info(void)
unreachable(); \
})
-#define schedule_tail(vcpu) (((vcpu)->arch.schedule_tail)(vcpu))
+/*
+ * Schedule tail *should* be a terminal function pointer, but leave a bugframe
+ * around just incase it returns, to save going back into the context
+ * switching code and leaving a far more subtle crash to diagnose.
+ */
+#define schedule_tail(vcpu) do { \
+ (((vcpu)->arch.schedule_tail)(vcpu)); \
+ BUG(); \
+ } while (0)
/*
* Which VCPU's state is currently running on each CPU?
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 49f7c0c..4ff89f0 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -395,7 +395,7 @@ struct arch_vcpu
unsigned long flags; /* TF_ */
- void noreturn (*schedule_tail) (struct vcpu *);
+ void (*schedule_tail) (struct vcpu *);
void (*ctxt_switch_from) (struct vcpu *);
void (*ctxt_switch_to) (struct vcpu *);
--
1.7.10.4
reply other threads:[~2014-03-07 11:05 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1394190308-2465-1-git-send-email-andrew.cooper3@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=keir@xen.org \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).