Re: [Xen-devel] [PATCH v6 2/4] mm: modify domain_adjust_tot_pages() to better handle a zero adjustment

From: Jan Beulich <jbeulich@suse.com>
To: Paul Durrant <pdurrant@amazon.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH v6 2/4] mm: modify domain_adjust_tot_pages() to better handle a zero adjustment
Date: Wed, 29 Jan 2020 16:07:39 +0100	[thread overview]
Message-ID: <a7342d93-84c3-d844-433e-c3d845c336c2@suse.com> (raw)
In-Reply-To: <20200129143831.1369-3-pdurrant@amazon.com>

On 29.01.2020 15:38, Paul Durrant wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -727,8 +727,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
>                               (j * (1UL << exch.out.extent_order)));
>  
>                  spin_lock(&d->page_alloc_lock);
> -                drop_dom_ref = (dec_count &&
> -                                !domain_adjust_tot_pages(d, -dec_count));
> +                drop_dom_ref = !domain_adjust_tot_pages(d, -dec_count);

And it's only now that I see it in this shape that it becomes
clear to me why the change above shouldn't be done, and why in
your other patch code should be written similar to the above:
The abstract model requires that the domain reference be
dropped only when ->tot_pages _transitions_ to zero. No drop
should occur if the count was already zero. Granted this may
be technically impossible in the specific case here, but the
code would still better reflect this general model, to prevent
it getting (mis-)cloned into other places.

Jan

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