Re: [Xen-devel] [PATCH v2 2/2] Xen/x86: Improve information from domain_crash_synchronous

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.09.13 at 14:46, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> +void asm_domain_crash_synchronous(unsigned long addr)
> +{
> +    if ( addr == 0 )
> +        addr = this_cpu(last_extable_addr);
> +
> +    printk("domain_crash_sync called from entry.S\n"
> +           "  fault at %p: ", _p(addr));
> +    print_symbol("%s\n", addr);

I'd prefer if all output went on a single line, so that grep-ing
though a log would turn up the fault locations. Perhaps the
"fault at" could go in parentheses at the end of the original
message?

>  #define UNLIKELY_START(cond, tag) \
> +        .Lunlikely.entry.tag:     \
>          j##cond .Lunlikely.tag;   \
>          .subsection 1;            \
>          .Lunlikely.tag:

I have to admit that I still dislike this dead label, albeit in the v2
shape it doesn't look as bad anymore. Nevertheless - why can't
you just use .Llikely.tag? That is in the original function, always
available (i.e. even - as done here - when using __UNLIKELY_END()),
and only very slightly off (pointing past the conditional branch
rather than at it).

And if we decided to stay with it, it still ask for it to be named
sensibly: It is not marking the entry of an unlikely code section
(as it sits in the "normal" code flow).

Jan


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