diff mbox series

[01/14] ram.c: Let the compress threads return a CompressResult enum

Message ID 18ad4a56517e3d63411e7cb8df6b47fe0162c406.1680457764.git.lukasstraub2@web.de (mailing list archive)
State New, archived
Headers show
Series migration/ram.c: Refactor compress code | expand

Commit Message

Lukas Straub April 2, 2023, 5:56 p.m. UTC 
This will be used in the next commits to move save_page_header()
out of compress code.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 migration/ram.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

--
2.30.2

Comments

Philippe Mathieu-Daudé April 3, 2023, 7:25 a.m. UTC | #1 
On 2/4/23 19:56, Lukas Straub wrote:
> This will be used in the next commits to move save_page_header()
> out of compress code.
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>   migration/ram.c | 34 ++++++++++++++++++++++------------
>   1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 9d1817ab7b..ca561e62bd 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -493,10 +493,17 @@ MigrationOps *migration_ops;
> 
>   CompressionStats compression_counters;
> 
> +enum CompressResult {
> +    RES_NONE = 0,

What about RES_INVALID?

> +    RES_ZEROPAGE = 1,
> +    RES_COMPRESS = 2
> +};


> -static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
> -                                 ram_addr_t offset, uint8_t *source_buf)
> +static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
> +                                           RAMBlock *block, ram_addr_t offset,
> +                                           uint8_t *source_buf)
>   {


>       if (ret < 0) {
>           qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
>           error_report("compressed data failed!");
> +        return RES_NONE;
>       }
> -    return false;
> +    return RES_COMPRESS;
>   }
Lukas Straub April 3, 2023, 10:59 a.m. UTC | #2 
On Mon, 3 Apr 2023 09:25:41 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 2/4/23 19:56, Lukas Straub wrote:
> > This will be used in the next commits to move save_page_header()
> > out of compress code.
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >   migration/ram.c | 34 ++++++++++++++++++++++------------
> >   1 file changed, 22 insertions(+), 12 deletions(-)
> > 
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 9d1817ab7b..ca561e62bd 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -493,10 +493,17 @@ MigrationOps *migration_ops;
> > 
> >   CompressionStats compression_counters;
> > 
> > +enum CompressResult {
> > +    RES_NONE = 0,  
> 
> What about RES_INVALID?

I think RES_NONE is more accurate, because having no result is a common
case. The submit side first handles the result from a previous
compression and then submits the new compression request. And for
example, when submitting the very first request to the thread there
won't be a previous result. Or when submitting after the threads where
flushed.

I just opted to return RES_NONE on error, because it seems more correct.

> > +    RES_ZEROPAGE = 1,
> > +    RES_COMPRESS = 2
> > +};  
> 
> 
> > -static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
> > -                                 ram_addr_t offset, uint8_t *source_buf)
> > +static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
> > +                                           RAMBlock *block, ram_addr_t offset,
> > +                                           uint8_t *source_buf)
> >   {  
> 
> 
> >       if (ret < 0) {
> >           qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> >           error_report("compressed data failed!");
> > +        return RES_NONE;
> >       }
> > -    return false;
> > +    return RES_COMPRESS;
> >   }  
> 



--
diff mbox series

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 9d1817ab7b..ca561e62bd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -493,10 +493,17 @@  MigrationOps *migration_ops;

 CompressionStats compression_counters;

+enum CompressResult {
+    RES_NONE = 0,
+    RES_ZEROPAGE = 1,
+    RES_COMPRESS = 2
+};
+typedef enum CompressResult CompressResult;
+
 struct CompressParam {
     bool done;
     bool quit;
-    bool zero_page;
+    CompressResult result;
     QEMUFile *file;
     QemuMutex mutex;
     QemuCond cond;
@@ -538,8 +545,9 @@  static QemuCond decomp_done_cond;

 static int ram_save_host_page_urgent(PageSearchStatus *pss);

-static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
-                                 ram_addr_t offset, uint8_t *source_buf);
+static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
+                                           RAMBlock *block, ram_addr_t offset,
+                                           uint8_t *source_buf);

 /* NOTE: page is the PFN not real ram_addr_t. */
 static void pss_init(PageSearchStatus *pss, RAMBlock *rb, ram_addr_t page)
@@ -564,7 +572,7 @@  static void *do_data_compress(void *opaque)
     CompressParam *param = opaque;
     RAMBlock *block;
     ram_addr_t offset;
-    bool zero_page;
+    CompressResult result;

     qemu_mutex_lock(&param->mutex);
     while (!param->quit) {
@@ -574,12 +582,12 @@  static void *do_data_compress(void *opaque)
             param->block = NULL;
             qemu_mutex_unlock(&param->mutex);

-            zero_page = do_compress_ram_page(param->file, &param->stream,
-                                             block, offset, param->originbuf);
+            result = do_compress_ram_page(param->file, &param->stream,
+                                          block, offset, param->originbuf);

             qemu_mutex_lock(&comp_done_lock);
             param->done = true;
-            param->zero_page = zero_page;
+            param->result = result;
             qemu_cond_signal(&comp_done_cond);
             qemu_mutex_unlock(&comp_done_lock);

@@ -1463,8 +1471,9 @@  static int ram_save_multifd_page(QEMUFile *file, RAMBlock *block,
     return 1;
 }

-static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
-                                 ram_addr_t offset, uint8_t *source_buf)
+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];
@@ -1472,7 +1481,7 @@  static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
     int ret;

     if (save_zero_page_to_file(pss, f, block, offset)) {
-        return true;
+        return RES_ZEROPAGE;
     }

     save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
@@ -1487,8 +1496,9 @@  static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
     if (ret < 0) {
         qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
         error_report("compressed data failed!");
+        return RES_NONE;
     }
-    return false;
+    return RES_COMPRESS;
 }

 static void
@@ -1496,7 +1506,7 @@  update_compress_thread_counts(const CompressParam *param, int bytes_xmit)
 {
     ram_transferred_add(bytes_xmit);

-    if (param->zero_page) {
+    if (param->result == RES_ZEROPAGE) {
         stat64_add(&ram_atomic_counters.duplicate, 1);
         return;
     }