On 2025/5/21 04:33, Peter Xu wrote:
On Tue, May 20, 2025 at 04:05:57PM -0300, Fabiano Rosas wrote:
Yanfei Xu<yanfei...@bytedance.com> writes:
There won't be any ram sync after the stage of save_complete, therefore
it's unnecessary to do manually protect for dirty pages being sent. Skip
to do this in last round can reduce noticeable downtime.
Signed-off-by: Yanfei Xu<yanfei...@bytedance.com>
---
As I don't have proper machine to test this patch in qemu and verify if it has
risks like in postcopy, colo and so on.(But I tested this idea on my rust VMM,
it works and can reduce ~50ms for a 128GB guest). So I raise the patch with RFC
for suggestions.
migration/ram.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index e12913b43e..2b169cdd18 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -838,7 +838,9 @@ static inline bool migration_bitmap_clear_dirty(RAMState
*rs,
* the page in the chunk we clear the remote dirty bitmap for all.
* Clearing it earlier won't be a problem, but too late will.
*/
- migration_clear_memory_region_dirty_bitmap(rb, page);
+ if (!rs->last_stage) {
+ migration_clear_memory_region_dirty_bitmap(rb, page);
+ }
ret = test_and_clear_bit(page, rb->bmap);
if (ret) {
Well, it looks ok to me and passes all my tests.
Tested-by: Fabiano Rosas<faro...@suse.de>
Reviewed-by: Fabiano Rosas<faro...@suse.de>
Thanks both!
I plan to test a bit on this patch later to see how much perf we can get in
QEMU. Since it makes perfect sense on its own, queued it for now, and the
plan is with below comments squashed in. Let me know if there's comments.
Postcopy unfortunately still cannot benefit much from this change, but I'll
prepare some patches soon so that this should ideally also work for the
whole lifecycle of postcopy. After that done, I am expecting some further
page fault latency reduction with this change.
Thanks for Fabiano's test and Peter's elaboration in comments.
Look foward to the performance data.
Regards, Yanfei
diff --git a/migration/ram.c b/migration/ram.c
index db70699f95..fd8d83b63c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -831,14 +831,20 @@ static inline bool migration_bitmap_clear_dirty(RAMState
*rs,
bool ret;
/*
- * Clear dirty bitmap if needed. This _must_ be called before we
- * send any of the page in the chunk because we need to make sure
- * we can capture further page content changes when we sync dirty
- * log the next time. So as long as we are going to send any of
- * the page in the chunk we clear the remote dirty bitmap for all.
- * Clearing it earlier won't be a problem, but too late will.
+ * During the last stage (after source VM stopped), resetting the write
+ * protections isn't needed as we know there will be either (1) no
+ * further writes if migration will complete, or (2) migration fails
+ * at last then tracking isn't needed either.
*/
if (!rs->last_stage) {
+ /*
+ * Clear dirty bitmap if needed. This _must_ be called before we
+ * send any of the page in the chunk because we need to make sure
+ * we can capture further page content changes when we sync dirty
+ * log the next time. So as long as we are going to send any of
+ * the page in the chunk we clear the remote dirty bitmap for all.
+ * Clearing it earlier won't be a problem, but too late will.
+ */
migration_clear_memory_region_dirty_bitmap(rb, page);
}
