qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 4/5] progressmeter: protect with a mutex


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v2 4/5] progressmeter: protect with a mutex
Date: Tue, 18 May 2021 13:00:38 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1

18.05.2021 12:40, Emanuele Giuseppe Esposito wrote:
Progressmeter is protected by the AioContext mutex, which
is taken by the block jobs and their caller (like blockdev).

We would like to remove the dependency of block layer code on the
AioContext mutex, since most drivers and the core I/O code are already
not relying on it.

Create a new C file to implement the ProgressMeter API, but keep the
struct as public, to avoid forcing allocation on the heap.

Also add a mutex to be able to provide an accurate snapshot of the
progress values to the caller.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

patch changed a lot, so you can't keep Stefan's r-b. r-b should be kept if 
patch is unchanged.


Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
  block/meson.build             |  1 +
  block/progress_meter.c        | 64 +++++++++++++++++++++++++++++++++++
  blockjob.c                    | 33 +++++++++++++-----
  include/qemu/progress_meter.h | 34 +++++++++++--------
  job-qmp.c                     |  8 +++--
  job.c                         |  3 ++
  qemu-img.c                    |  9 +++--
  7 files changed, 124 insertions(+), 28 deletions(-)
  create mode 100644 block/progress_meter.c

diff --git a/block/meson.build b/block/meson.build
index d21990ec95..90efd21ecf 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -13,6 +13,7 @@ block_ss.add(files(
    'commit.c',
    'copy-on-read.c',
    'preallocate.c',
+  'progress_meter.c',
    'create.c',
    'crypto.c',
    'dirty-bitmap.c',
diff --git a/block/progress_meter.c b/block/progress_meter.c
new file mode 100644
index 0000000000..aa2e60248c
--- /dev/null
+++ b/block/progress_meter.c
@@ -0,0 +1,64 @@
+/*
+ * Helper functionality for some process progress tracking.
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012, 2018 Red Hat, Inc.
+ * Copyright (c) 2020 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "qemu/progress_meter.h"
+
+void progress_init(ProgressMeter *pm)
+{
+    qemu_mutex_init(&pm->lock);
+}
+
+void progress_destroy(ProgressMeter *pm)
+{
+    qemu_mutex_destroy(&pm->lock);
+}
+
+void progress_get_snapshot(ProgressMeter *pm, uint64_t *current,
+                           uint64_t *total)
+{
+    QEMU_LOCK_GUARD(&pm->lock);
+
+    *current = pm->current;
+    *total = pm->total;
+}
+
+void progress_work_done(ProgressMeter *pm, uint64_t done)
+{
+    QEMU_LOCK_GUARD(&pm->lock);
+    pm->current += done;
+}
+
+void progress_set_remaining(ProgressMeter *pm, uint64_t remaining)
+{
+    QEMU_LOCK_GUARD(&pm->lock);
+    pm->total = pm->current + remaining;
+}
+
+void progress_increase_remaining(ProgressMeter *pm, uint64_t delta)
+{
+    QEMU_LOCK_GUARD(&pm->lock);
+    pm->total += delta;
+}

That's not what I meant. If we move only function to c file, this doesn't make 
real sense. If we want encapsulation, we should move the structure definition 
itself to the c file too. Than, you'll need _new / _free functions instead of 
_init / _destroy, because we'll move to heap allocation for progress meter. 
Also, if we go this way, please make at least two patches:

1. move progress meter (functions and structure) to own c file, modify 
meson.build

2. add thread-safety





--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]