From: Wei Liu <wei.liu2@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, David Scott <dave@recoil.org>,
Ian Jackson <Ian.Jackson@eu.citrix.com>,
Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH for-4.8] tools/oxenstored: Fix transaction handling in 32bit builds
Date: Mon, 31 Oct 2016 14:19:20 +0000 [thread overview]
Message-ID: <20161031141920.GX30231@citrix.com> (raw)
In-Reply-To: <1477920116-16768-1-git-send-email-andrew.cooper3@citrix.com>
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>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-10-31 14:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-31 13:21 [PATCH for-4.8] tools/oxenstored: Fix transaction handling in 32bit builds Andrew Cooper
2016-10-31 14:19 ` Wei Liu [this message]
2016-10-31 20:51 ` David Scott
2016-10-31 21:03 ` Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161031141920.GX30231@citrix.com \
--to=wei.liu2@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=dave@recoil.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).