Re: [PATCH v6 09/12] softmmu/physmem: Don't use atomic operations in ram_block_discard_(disable|require)

From: Paolo Bonzini <pbonzini@redhat.com>
To: David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Wei Yang <richard.weiyang@linux.alibaba.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Peter Xu <peterx@redhat.com>, Auger Eric <eric.auger@redhat.com>,
	Pankaj Gupta <pankaj.gupta@cloud.ionos.com>,
	teawater <teawaterz@linux.alibaba.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Marek Kedzierski <mkedzier@redhat.com>
Subject: Re: [PATCH v6 09/12] softmmu/physmem: Don't use atomic operations in ram_block_discard_(disable|require)
Date: Mon, 22 Feb 2021 15:02:08 +0100	[thread overview]
Message-ID: <61237335-b03f-cb89-c0be-03fc3058c13b@redhat.com> (raw)
In-Reply-To: <d64fdd53-6868-15fd-ea81-fa26e153eced@redhat.com>

On 22/02/21 14:33, David Hildenbrand wrote:
>> Also, uncoordinated require is unused, and therefore uncoordinated
>> disable is also never going to block anything.  Does it make sense to
>> keep it in the API?
> 
> Right, "ram_block_discard_require()" is not used yet. I am planning on 
> using it in virtio-balloon context at some point, but can remove it for 
> now to simplify.
> 
> ram_block_uncoordinated_discard_disable(), however, will block 
> virtio-balloon already via ram_block_discard_is_disabled(). (yes, 
> virtio-balloon is ugly)

Oops, I missed that API.

Does it make sense to turn the API inside out, with the 
coordinated/uncoordinated choice as an argument and the start/finish 
choice in the name?

enum {
     RAM_DISCARD_ALLOW_COORDINATED = 1,
};

bool ram_discard_disable(int flags, Error **errp);
void ram_discard_enable(int flags);
int ram_discard_start(bool coordinated, Error **errp);
void ram_discard_finish(bool coordinated);

Paolo