All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.