Re: [Xen-devel] [PATCH] x86/time: fix scale_delta() inline assembly

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

> From: Keir Fraser [mailto:keir@xxxxxxx]
> Sent: Monday, November 26, 2012 9:11 AM
> To: Jan Beulich; xen-devel
> Subject: Re: [Xen-devel] [PATCH] x86/time: fix scale_delta() inline assembly
> 
> On 26/11/2012 15:23, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
> > The way it was coded, it clobbered %rdx without telling the compiler.
> > This generally didn't cause any problems except when there are two back
> > to back invocations (as in plt_overflow()), as in that case the
> > compiler may validly assume that it can re-use for the second instance
> > the value loaded into %rdx before the first one.
> >
> > Once at it, also properly relax the second operand of "mul" (there's no
> > need for it to be in %rdx, or a register at all), and switch away from
> > using explicit register names in the instruction operands.
> >
> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Ugh. Thank you very much, Jan! What a horrible bug.
> 
> Acked-by: Keir Fraser <keir@xxxxxxx>

Amazing work Jan!

Suggestion (especially since this is DOCDAY)...

Since this has caused a number of people a lot of pain
over a period of years, might it make sense to very
clearly call out the user-visible problem(s) fixed by this
in the commit message -- possibly including a few
xen-devel archive URLs and/or subject lines?  Other
than the reference to plt_overflow() (and the fact that
I've been following this thread for years), I wouldn't
have any idea from the commit message that this bug
fixes the "clock shifts by 50min" problem.

It would probably also be nice to cc the various people
who have contributed their time to provide supporting
information that helped isolate the bug.

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