[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

On 4/15/2013 12:50 PM, Ian Jackson wrote:
Ian Jackson writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include 
outstanding_claims value."):
Konrad Rzeszutek Wilk writes ("[PATCH 7/7] xl: Fix 'free_memory' to include 
outstanding_claims value."):
Updating to make it clear that free_memory reported by 'xl info'
is influenced by the outstanding claim value. [...]
      if (libxl_get_physinfo(ctx, &info) != 0) {
          fprintf(stderr, "libxl_physinfo failed.\n");
+    /*
+     * Don't bother checking "claim_mode" as user might have turned it off
+     * and we have outstanding claims.
+     */
+    if ((claims = libxl_get_claiminfo(ctx)) < 0){
+        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
+                errno, strerror(errno));
+        return;
+    }
+        printf("free_memory            : %"PRIu64"\n", (info.free_pages - 
claims) / i);
This has a race, I think.
I have checked this and the race is in the hypercall API.  The
hypercall API has already been checked in.  So, under the
circumstances, for this patch:

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Great! Thanks.
However, there is a release-critical bug here.  The following things
need to be changed:

  * We need a race-free version of the hypercall API.
  * We need a race-free version of the xc API.
  * We need a race-free version of the libxl API.

I think this is a release-critical bug because fixing it involves an
API change at all these 3 levels.  We don't want to release 4.3 with
a broken API as that will complicate fixing this bug.

Right. Let me enumerate some ideas for fixing this on a different thread (if we have the same race in mind that you spotted).

George, can you please add this to your tracking list ?

Having said all that, George, am I OK from a freeze POV to pull
Konrad's series into staging ?

Would you like me to rebase it once more with the s/def_bool/int/ and the resurrection of a line change? I can repost just against the offending patch?

I might be a bit late with doing it (Tuesday night) since I am traveling today.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.