Re: [PATCH v2 2/6] sched: Remove dependency on __LINE__ for release builds

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2499 bytes --]

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
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel