On 25/05/2023 14:16, Peter Xu wrote: > On Thu, May 25, 2023 at 12:43:20PM +0100, Joao Martins wrote: >> In preparation for including the number of dirty pages in the >> vfio_get_dirty_bitmap() tracepoint, return the number of dirty pages in >> cpu_physical_memory_set_dirty_lebitmap() similar to >> cpu_physical_memory_sync_dirty_bitmap(). > > The patch itself looks good to me, but it's slightly different from sync > version because that was only for MIGRATION bitmap, meanwhile it counts > newly dirtied ones (so exclude already dirtied ones even if re-dirtied in > the MIGRATION bitmap), while this one counts any dirty bits in *bitmap. > Good callout.
> Shall we perhaps state it somewhere explicitly? A comment for retval might > be suitable above the function? > Yeap, Something like this? diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 8b8f271d0731..deaf746421da 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -333,6 +333,13 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, } #if !defined(_WIN32) + +/* + * Contrary to cpu_physical_memory_sync_dirty_bitmap() this function returns + * the number of dirty pages in @bitmap passed as argument. On the other hand, + * cpu_physical_memory_sync_dirty_bitmap() returns newly dirtied pages that + * weren't set in the global migration bitmap. + */ static inline uint64_t cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, ram_addr_t start,