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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.09.13 at 15:49, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/09/13 14:45, Jan Beulich wrote:
>>>>> 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?
> 
> Certainly - I shall respin.
> 
>>
>>>  #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).
> 
> I suppose pointing at the end of the unlikely section is ok, but I still
> prefer pointing to the actual instruction which made the decsion.

Just so we talk the same "language" - to me "unlikely section"
means the portion of moved out of the normal code flow. And
using .Llikely.tag you'd already not have the label point into the
unlikely section (for, as we both appear to agree on, it being
slightly more involved to re-associate that out-of-line location
with the origin). All we diverge on is whether the label points
at the beginning or the end of the branch instruction sitting in
the normal code flow.

> What name would you suggest? I admit that UNLIKELY_ENTRY_LABEL() is not
> the best name but I couldn't think of a better name.

As per above, my favorite would be to not have another label
at all. Failing that, .Ldispatch.tag would come to mind. All I'm
asking for is that the label name not cause confusion through
spelling out something that it doesn't fulfill.

Jan


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