[Top][All Lists]

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

Re: [PATCH v6 09/12] softmmu/physmem: Don't use atomic operations in ram

From: Paolo Bonzini
Subject: Re: [PATCH v6 09/12] softmmu/physmem: Don't use atomic operations in ram_block_discard_(disable|require)
Date: Mon, 22 Feb 2021 18:32:16 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 22/02/21 16:38, David Hildenbrand wrote:
On 22.02.21 15:02, Paolo Bonzini wrote:
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 {

Any reason to go with an enum/flags for this case and not "bool allow_coordinated" ?

I find it slightly easier to remember the meaning of true for "bool coordinated" than for "bool allow_coordinated". I don't like the API below that much, but having both RAM_DISCARD_ALLOW_COORDINATED for disable/enable and RAM_DISCARD_SUPPORT_COORDINATED for start/finish would be even uglier...


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);

Yeah, I tried to avoid boolean flags ;) Don't have a strong opinion. At least we get shorter names.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]