qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-devel@nongnu.org, qemu-block@nongnu.org
Cc: kwolf@redhat.com, mreitz@redhat.com, berrange@redhat.com,
	berto@igalia.com, vsementsov@virtuozzo.com, den@openvz.org,
	pbonzini@redhat.com
Subject: [Qemu-devel] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking
Date: Tue,  8 Jan 2019 20:06:52 +0300	[thread overview]
Message-ID: <20190108170655.29766-7-vsementsov@virtuozzo.com> (raw)
In-Reply-To: <20190108170655.29766-1-vsementsov@virtuozzo.com>

Background: decryption will be done in threads, to take benefit of it,
we should move it out of the lock first.

But let's go further: it turns out, that for locking around switch
cases we have only two variants: when we just do memset(0) not
releasing the lock (it is useless) and when we actually can handle the
whole case out of the lock. So, refactor the whole thing to reduce
locked code region and make it clean.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2865903b0e..e650528af7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1883,6 +1883,7 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 
         ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, &cluster_offset);
         if (ret < 0) {
+            qemu_co_mutex_unlock(&s->lock);
             goto fail;
         }
 
@@ -1891,39 +1892,38 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
         qemu_iovec_reset(&hd_qiov);
         qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
 
+        if (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
+            ret == QCOW2_CLUSTER_ZERO_ALLOC ||
+            (ret == QCOW2_CLUSTER_UNALLOCATED && !bs->backing))
+        {
+            /* No sense in releasing the lock */
+
+            qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
+
+            bytes -= cur_bytes;
+            offset += cur_bytes;
+            bytes_done += cur_bytes;
+            continue;
+        }
+
+        qemu_co_mutex_unlock(&s->lock);
+
         switch (ret) {
         case QCOW2_CLUSTER_UNALLOCATED:
-
-            if (bs->backing) {
-                BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-                qemu_co_mutex_unlock(&s->lock);
-                ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
-                                     &hd_qiov, 0);
-                qemu_co_mutex_lock(&s->lock);
-                if (ret < 0) {
-                    goto fail;
-                }
-            } else {
-                /* Note: in this case, no need to wait */
-                qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
+            BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
+            ret = bdrv_co_preadv(bs->backing, offset, cur_bytes, &hd_qiov, 0);
+            if (ret < 0) {
+                goto fail;
             }
             break;
 
-        case QCOW2_CLUSTER_ZERO_PLAIN:
-        case QCOW2_CLUSTER_ZERO_ALLOC:
-            qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes);
-            break;
-
         case QCOW2_CLUSTER_COMPRESSED:
-            qemu_co_mutex_unlock(&s->lock);
             ret = qcow2_co_preadv_compressed(bs, cluster_offset,
                                              offset, cur_bytes,
                                              &hd_qiov);
-            qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 goto fail;
             }
-
             break;
 
         case QCOW2_CLUSTER_NORMAL:
@@ -1956,11 +1956,9 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
             }
 
             BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-            qemu_co_mutex_unlock(&s->lock);
             ret = bdrv_co_preadv(bs->file,
                                  cluster_offset + offset_in_cluster,
                                  cur_bytes, &hd_qiov, 0);
-            qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 goto fail;
             }
@@ -1991,12 +1989,14 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
         bytes -= cur_bytes;
         offset += cur_bytes;
         bytes_done += cur_bytes;
+
+        qemu_co_mutex_lock(&s->lock);
     }
     ret = 0;
 
-fail:
     qemu_co_mutex_unlock(&s->lock);
 
+fail:
     qemu_iovec_destroy(&hd_qiov);
     qemu_vfree(cluster_data);
 
-- 
2.18.0

  parent reply	other threads:[~2019-01-08 17:07 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-08 17:06 [Qemu-devel] [PATCH v3 0/9] qcow2: encryption threads Vladimir Sementsov-Ogievskiy
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 1/9] qcow2.h: add missing include Vladimir Sementsov-Ogievskiy
2019-01-09 10:32   ` Alberto Garcia
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 2/9] qcow2: add separate file for threaded data processing functions Vladimir Sementsov-Ogievskiy
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 3/9] qcow2-threads: use thread_pool_submit_co Vladimir Sementsov-Ogievskiy
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 4/9] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex Vladimir Sementsov-Ogievskiy
2019-01-15 23:29   ` Paolo Bonzini
2019-01-22 10:22     ` Vladimir Sementsov-Ogievskiy
2019-01-16 13:11   ` Alberto Garcia
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 5/9] qcow2-threads: split out generic path Vladimir Sementsov-Ogievskiy
2019-01-16 13:14   ` Alberto Garcia
2019-01-08 17:06 ` Vladimir Sementsov-Ogievskiy [this message]
2019-01-16 13:53   ` [Qemu-devel] [PATCH v3 6/9] qcow2: qcow2_co_preadv: improve locking Alberto Garcia
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 7/9] qcow2: qcow2_co_preadv: skip using hd_qiov when possible Vladimir Sementsov-Ogievskiy
2019-01-16 16:18   ` Alberto Garcia
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 8/9] qcow2: bdrv_co_pwritev: move encryption code out of the lock Vladimir Sementsov-Ogievskiy
2019-01-18  9:51   ` Alberto Garcia
2019-01-18 11:37     ` Vladimir Sementsov-Ogievskiy
2019-01-18 14:39       ` Alberto Garcia
2019-01-18 17:15         ` Vladimir Sementsov-Ogievskiy
2019-01-08 17:06 ` [Qemu-devel] [PATCH v3 9/9] qcow2: do encryption in threads Vladimir Sementsov-Ogievskiy
2019-01-18 12:41   ` Alberto Garcia
2019-01-23 14:54 ` [Qemu-devel] [PATCH v3 0/9] qcow2: encryption threads no-reply
2019-01-23 15:07   ` Vladimir Sementsov-Ogievskiy
2019-01-23 15:38     ` Eric Blake
2019-01-28 11:54 ` [Qemu-devel] ping " Vladimir Sementsov-Ogievskiy

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=20190108170655.29766-7-vsementsov@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=berrange@redhat.com \
    --cc=berto@igalia.com \
    --cc=den@openvz.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).