Re: [PATCH for-4.8] tools/oxenstored: Fix transaction handling in 32bit builds

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Oct 31, 2016 at 08:51:07PM +0000, David Scott wrote:
> 
> > On 31 Oct 2016, at 14:19, Wei Liu <wei.liu2@citrix.com> wrote:
> > 
> > On Mon, Oct 31, 2016 at 01:21:56PM +0000, Andrew Cooper wrote:
> >> In a 32bit build, the ocaml code 'proposed_id >= 0x7fffffff' compiles to:
> >> 
> >>  8055eac:       83 fb ff                cmp    $0xffffffff,%ebx
> >>  8055eaf:       7d 0f                   jge    8055ec0 <...+0x20>
> >> 
> >> which in C is 'proposed_id >= INT_MIN', or in other words, tautologically
> >> true.  As a result, 32bit builds of oxenstored always try to allocate the
> >> transaction id 1, and fall into an infinite loop of trying the next id if
> >> transaction 1 is already in use.
> >> 
> >> Restrict the range down to 1 billion, to sit in the positive half of a 31 bit
> >> ocaml integer.  The compiled code is now:
> >> 
> >>  8055eac:       b9 ff ff ff 7f          mov    $0x7fffffff,%ecx
> >>  8055eb1:       39 cb                   cmp    %ecx,%ebx
> >>  8055eb3:       7d 0b                   jge    8055ec0 <...+0x20>
> >> 
> >> which (other than non-optimal code generation because of the unnecessary use
> >> of %ecx), isn't unconditionally true.
> >> 
> >> In principle, the check could be changed to 'proposed_id == 0x7fffffff' which
> >> would still allow for 2 billion transaction in 32bit builds.  However, in
> >> 64bit builds, this reintroduces a risk that if proposed_id is initially
> >> greater than 0x7fffffff, it will not be clipped suitably into range.
> >> 
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> 
> Acked-by: David Scott <dave@recoil.org>
> 

Applied. Thanks.

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