From mboxrd@z Thu Jan  1 00:00:00 1970
From: konrad wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH 7/7] xl: Fix 'free_memory' to include
 outstanding_claims value.
Date: Mon, 15 Apr 2013 20:19:13 -0400
Message-ID: <516C9901.30706@oracle.com>
References: <1365800181-9877-1-git-send-email-konrad.wilk@oracle.com>
	<1365800181-9877-8-git-send-email-konrad.wilk@oracle.com>
	<20843.59271.672380.158979@mariner.uk.xensource.com>
	<20844.12237.939778.932762@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20844.12237.939778.932762@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>, Dan Magenheimer <dan.magenheimer@oracle.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org


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");
>>>           return;
>>>       }
>>> -
>>> +    /*
>>> +     * 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@eu.citrix.com>
>

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.
>
> Ian.