| Message ID | 1488995215-7647-3-git-send-email-ross.lagerwall@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-03-08 at 17:46 +0000, Ross Lagerwall wrote: > When using LivePatch, use of __LINE__ can generate spurious changes > in > functions due to embedded line numbers. For release builds with > LivePatch enabled, remove the use of these line numbers in > domain_crash*() and print the current text address instead. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > This patch looks good to me. I wonder, though... > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -625,20 +625,28 @@ void vcpu_end_shutdown_deferral(struct vcpu > *v); > * from any processor. > */ > void __domain_crash(struct domain *d); > -#define domain_crash(d) do > { \ > - printk("domain_crash called from %s:%d\n", __FILE__, > __LINE__); \ > - __domain_crash(d); > \ > + > +#if defined(NDEBUG) && defined(CONFIG_LIVEPATCH) > +#define _domain_crash(func, call) do > { \ > + printk(#func " called from %pS\n", > current_text_addr()); \ > + call; > \ > +} while (0) > +#else > +#define _domain_crash(func, call) do > { \ > + printk(#func " called from %s:%d\n", __FILE__, > __LINE__); \ > + call; > \ > } while (0) > +#endif > + ...if it's really worth providing both the alternatives. It's more code, it's --to some extent-- code duplication, for very little gain. Actually, it even results in different output depending on configure options, which is not the end of the world, but not super nice either, IMO. I'd say we would be better of with having just the former macro in all cases. Thanks and Regards, Dario
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 0929c0b..f385de3 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -625,20 +625,28 @@ void vcpu_end_shutdown_deferral(struct vcpu *v); * from any processor. */ void __domain_crash(struct domain *d); -#define domain_crash(d) do { \ - printk("domain_crash called from %s:%d\n", __FILE__, __LINE__); \ - __domain_crash(d); \ + +#if defined(NDEBUG) && defined(CONFIG_LIVEPATCH) +#define _domain_crash(func, call) do { \ + printk(#func " called from %pS\n", current_text_addr()); \ + call; \ +} while (0) +#else +#define _domain_crash(func, call) do { \ + printk(#func " called from %s:%d\n", __FILE__, __LINE__); \ + call; \ } while (0) +#endif + +#define domain_crash(d) _domain_crash(domain_crash, __domain_crash(d)) /* * Mark current domain as crashed and synchronously deschedule from the local * processor. This function never returns. */ void noreturn __domain_crash_synchronous(void); -#define domain_crash_synchronous() do { \ - printk("domain_crash_sync called from %s:%d\n", __FILE__, __LINE__); \ - __domain_crash_synchronous(); \ -} while (0) +#define domain_crash_synchronous() \ + _domain_crash(domain_crash_sync, __domain_crash_synchronous()) /* * Called from assembly code, with an optional address to help indicate why
When using LivePatch, use of __LINE__ can generate spurious changes in functions due to embedded line numbers. For release builds with LivePatch enabled, remove the use of these line numbers in domain_crash*() and print the current text address instead. Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> --- Changes in v2: * Simply macros. * Use %pS. xen/include/xen/sched.h | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)