All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: qemu-devel@nongnu.org
Cc: Thomas Huth <thuth@redhat.com>, Peter Xu <peterx@redhat.com>,
	Leonardo Bras <leobras@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	Laurent Vivier <lvivier@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Lukas Straub <lukasstraub2@web.de>
Subject: [PULL 06/13] ram.c: Do not call save_page_header() from compress threads
Date: Mon,  8 May 2023 20:52:02 +0200	[thread overview]
Message-ID: <20230508185209.68604-7-quintela@redhat.com> (raw)
In-Reply-To: <20230508185209.68604-1-quintela@redhat.com>

From: Lukas Straub <lukasstraub2@web.de>

save_page_header() accesses several global variables, so calling it
from multiple threads is pretty ugly.

Instead, call save_page_header() before writing out the compressed
data from the compress buffer to the migration stream.

This also makes the core compress code more independend from ram.c.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 4e14e3bb94..c52602b70d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1465,17 +1465,13 @@ static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
                                            RAMBlock *block, ram_addr_t offset,
                                            uint8_t *source_buf)
 {
-    RAMState *rs = ram_state;
-    PageSearchStatus *pss = &rs->pss[RAM_CHANNEL_PRECOPY];
     uint8_t *p = block->host + offset;
     int ret;
 
-    if (save_zero_page_to_file(pss, f, block, offset)) {
+    if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
         return RES_ZEROPAGE;
     }
 
-    save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
-
     /*
      * copy it to a internal buffer to avoid it being modified by VM
      * so that we can catch up the error during compression and
@@ -1515,9 +1511,40 @@ static inline void compress_reset_result(CompressParam *param)
     param->offset = 0;
 }
 
+static int send_queued_data(CompressParam *param)
+{
+    PageSearchStatus *pss = &ram_state->pss[RAM_CHANNEL_PRECOPY];
+    MigrationState *ms = migrate_get_current();
+    QEMUFile *file = ms->to_dst_file;
+    int len = 0;
+
+    RAMBlock *block = param->block;
+    ram_addr_t offset = param->offset;
+
+    if (param->result == RES_NONE) {
+        return 0;
+    }
+
+    assert(block == pss->last_sent_block);
+
+    if (param->result == RES_ZEROPAGE) {
+        len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
+        qemu_put_byte(file, 0);
+        len += 1;
+        ram_release_page(block->idstr, offset);
+    } else if (param->result == RES_COMPRESS) {
+        len += save_page_header(pss, file, block,
+                                offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
+        len += qemu_put_qemu_file(file, param->file);
+    } else {
+        abort();
+    }
+
+    return len;
+}
+
 static void flush_compressed_data(RAMState *rs)
 {
-    MigrationState *ms = migrate_get_current();
     int idx, len, thread_count;
 
     if (!save_page_use_compression(rs)) {
@@ -1537,7 +1564,7 @@ static void flush_compressed_data(RAMState *rs)
         qemu_mutex_lock(&comp_param[idx].mutex);
         if (!comp_param[idx].quit) {
             CompressParam *param = &comp_param[idx];
-            len = qemu_put_qemu_file(ms->to_dst_file, param->file);
+            len = send_queued_data(param);
             compress_reset_result(param);
 
             /*
@@ -1563,7 +1590,6 @@ static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset)
 {
     int idx, thread_count, bytes_xmit = -1, pages = -1;
     bool wait = migrate_compress_wait_thread();
-    MigrationState *ms = migrate_get_current();
 
     thread_count = migrate_compress_threads();
     qemu_mutex_lock(&comp_done_lock);
@@ -1573,7 +1599,7 @@ retry:
             CompressParam *param = &comp_param[idx];
             qemu_mutex_lock(&param->mutex);
             param->done = false;
-            bytes_xmit = qemu_put_qemu_file(ms->to_dst_file, param->file);
+            bytes_xmit = send_queued_data(param);
             compress_reset_result(param);
             set_compress_params(param, block, offset);
 
-- 
2.40.0



  parent reply	other threads:[~2023-05-08 18:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-08 18:51 [PULL 00/13] Compression code patches Juan Quintela
2023-05-08 18:51 ` [PULL 01/13] qtest/migration-test.c: Add tests with compress enabled Juan Quintela
2023-05-08 18:51 ` [PULL 02/13] qtest/migration-test.c: Add postcopy " Juan Quintela
2023-05-08 18:51 ` [PULL 03/13] ram.c: Let the compress threads return a CompressResult enum Juan Quintela
2023-05-08 18:52 ` [PULL 04/13] ram.c: Dont change param->block in the compress thread Juan Quintela
2023-05-08 18:52 ` [PULL 05/13] ram.c: Reset result after sending queued data Juan Quintela
2023-05-08 18:52 ` Juan Quintela [this message]
2023-05-08 18:52 ` [PULL 07/13] ram.c: Call update_compress_thread_counts from compress_send_queued_data Juan Quintela
2023-05-08 18:52 ` [PULL 08/13] ram.c: Remove last ram.c dependency from the core compress code Juan Quintela
2023-05-08 18:52 ` [PULL 09/13] ram.c: Move core compression code into its own file Juan Quintela
2023-05-08 18:52 ` [PULL 10/13] ram.c: Move core decompression " Juan Quintela
2023-05-08 18:52 ` [PULL 11/13] ram compress: Assert that the file buffer matches the result Juan Quintela
2023-05-08 18:52 ` [PULL 12/13] ram-compress.c: Make target independent Juan Quintela
2023-05-08 18:52 ` [PULL 13/13] migration: Initialize and cleanup decompression in migration.c Juan Quintela
2023-05-09  6:34 ` [PULL 00/13] Compression code patches Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230508185209.68604-7-quintela@redhat.com \
    --to=quintela@redhat.com \
    --cc=leobras@redhat.com \
    --cc=lukasstraub2@web.de \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.