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,

Reply via email to