Re: [PATCH 2/7] guest_memfd: Introduce an object to manage the guest-memfd with RamDiscardManager

Chenyi Qiang
2025-01-21T09:01:04Z

Thanks Peter for your review!

On 1/21/2025 2:09 AM, Peter Xu wrote:
> Two trivial comments I spot:
>
> On Fri, Dec 13, 2024 at 03:08:44PM +0800, Chenyi Qiang wrote:
>> +struct GuestMemfdManager {
>> + Object parent;
>> +
>> + /* Managed memory region. */
>> + MemoryRegion *mr;
>> +
>> + /*
>> + * 1-setting of the bit represents the memory is populated (shared).
>> + */
>> + int32_t bitmap_size;
>> + unsigned long *bitmap;
>
> Might be clearer to name the bitmap directly as what it represents. E.g.,
> shared_bitmap?

Make sense.

>
>> +
>> + /* block size and alignment */
>> + uint64_t block_size;
>
> Can we always fetch it from the MR/ramblock? If this is needed, better add
> some comment explaining why.

The block_size is the granularity used to track the private/shared
attribute in the bitmap. It is currently hardcoded to 4K as guest_memfd
may manipulate the page conversion in at least 4K size and alignment.
I think It is somewhat a variable to cache the size and can avoid many
getpagesize() calls.

>
>> +
>> + /* listeners to notify on populate/discard activity. */
>> + QLIST_HEAD(, RamDiscardListener) rdl_list;
>> +};
>