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

[Chenyi Qiang]

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;
>> +};
>