[v1] Don't sync volatile memory

[Patch] Don't sync volatile memory

Posted by Chaney, Ben 5 months ago

Not all pmem regions are backed by non-volatile memory. Syncing volatile
memory provides no benefit, but can cause performance issues is some
cases. Only sync memory that is marked as non-volatile.

Fixes: bd108a44bc29 (migration: ram: Switch to ram block writeback)

---
migration/ram.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index f25ebd9620..e3ab67acd4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3930,7 +3930,9 @@ static int ram_load_cleanup(void *opaque)
     RAMBlock *rb;

     RAMBLOCK_FOREACH_NOT_IGNORED(rb) {
-        qemu_ram_block_writeback(rb);
+        if (memory_region_is_nonvolatile(rb->mr)) {
+            qemu_ram_block_writeback(rb);
+        }
     }

     xbzrle_load_cleanup();
--
2.40.1

Re: [Patch] Don't sync volatile memory

Posted by Peter Xu 5 months ago

On Mon, Jun 16, 2025 at 07:43:02PM +0000, Chaney, Ben wrote:
> Not all pmem regions are backed by non-volatile memory. Syncing volatile
> memory provides no benefit, but can cause performance issues is some
> cases. Only sync memory that is marked as non-volatile.
> 
> Fixes: bd108a44bc29 (migration: ram: Switch to ram block writeback)

Do you plan to work on qemu in the future?  If so, I suggest you go
through this and repost:

https://www.qemu.org/docs/master/devel/submitting-a-patch.html

If the answer is no, I'll at least need your Signed-off-By: line here, then
I can fix all the rest for you.  The patch looks ok though.

Thanks,

> 
> ---
> migration/ram.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index f25ebd9620..e3ab67acd4 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3930,7 +3930,9 @@ static int ram_load_cleanup(void *opaque)
>      RAMBlock *rb;
> 
>      RAMBLOCK_FOREACH_NOT_IGNORED(rb) {
> -        qemu_ram_block_writeback(rb);
> +        if (memory_region_is_nonvolatile(rb->mr)) {
> +            qemu_ram_block_writeback(rb);
> +        }
>      }
> 
>      xbzrle_load_cleanup();
> --
> 2.40.1
> 
> 
> 
> 

-- 
Peter Xu

Re: [Patch] Don't sync volatile memory

Posted by Chaney, Ben 5 months ago


On 6/16/25, 4:35 PM, "Peter Xu" <peterx@redhat.com <mailto:peterx@redhat.com>> wrote:

> On Mon, Jun 16, 2025 at 07:43:02PM +0000, Chaney, Ben wrote:
>> Not all pmem regions are backed by non-volatile memory. Syncing volatile
>> memory provides no benefit, but can cause performance issues is some
>> cases. Only sync memory that is marked as non-volatile.
>>
>> Fixes: bd108a44bc29 (migration: ram: Switch to ram block writeback)
>
> Do you plan to work on qemu in the future? If so, I suggest you go
> through this and repost:

Hi Peter,
        I resent the patch with a Signed-off-By: line and rebased against master.

        Please let me know if anything else is needed.
Thanks,
        Ben