All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pekka Enberg <penberg@kernel.org>
To: kvm@vger.kernel.org
Cc: Pekka Enberg <penberg@kernel.org>,
	Asias He <asias.hejun@gmail.com>,
	Cyrill Gorcunov <gorcunov@gmail.com>, Ingo Molnar <mingo@elte.hu>,
	Prasad Joshi <prasadjoshi124@gmail.com>,
	Sasha Levin <levinsasha928@gmail.com>
Subject: [PATCH 3/9] kvm tools, qcow: Fix locking issues
Date: Sat,  9 Jul 2011 16:02:36 +0300	[thread overview]
Message-ID: <1310216563-17503-4-git-send-email-penberg@kernel.org> (raw)
In-Reply-To: <1310216563-17503-1-git-send-email-penberg@kernel.org>

The virtio_blk_do_io() function can enter the QCOW code through
disk_image__{read,write,flush}() from multiple threads because it uses a thread
pool for I/O requests. Thus, use locking to make the QCOW2 code thread-safe.

Cc: Asias He <asias.hejun@gmail.com>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Prasad Joshi <prasadjoshi124@gmail.com>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/disk/qcow.c         |   30 +++++++++++++++++++++---------
 tools/kvm/include/kvm/mutex.h |    6 ++++++
 tools/kvm/include/kvm/qcow.h  |    3 +++
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index a1f6ef3..939bc61 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -2,6 +2,7 @@
 
 #include "kvm/disk-image.h"
 #include "kvm/read-write.h"
+#include "kvm/mutex.h"
 #include "kvm/util.h"
 
 #include <sys/types.h>
@@ -232,16 +233,17 @@ static ssize_t qcow_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst_
 
 	l1_idx = get_l1_index(q, offset);
 	if (l1_idx >= table->table_size)
-		goto out_error;
+		return -1;
 
 	clust_offset = get_cluster_offset(q, offset);
 	if (clust_offset >= cluster_size)
-		goto out_error;
+		return -1;
 
 	length = cluster_size - clust_offset;
 	if (length > dst_len)
 		length = dst_len;
 
+	mutex_lock(&q->mutex);
 	l2_table_offset = table->l1_table[l1_idx] & ~header->oflag_mask;
 	if (!l2_table_offset)
 		goto zero_cluster;
@@ -261,19 +263,22 @@ static ssize_t qcow_read_cluster(struct qcow *q, u64 offset, void *dst, u32 dst_
 	if (!clust_start)
 		goto zero_cluster;
 
+	mutex_unlock(&q->mutex);
+
 	if (pread_in_full(q->fd, dst, length, clust_start + clust_offset) < 0)
-		goto out_error;
+		return -1;
 
-out:
 	return length;
 
 zero_cluster:
+	mutex_unlock(&q->mutex);
 	memset(dst, 0, length);
-	goto out;
+	return length;
 
 out_error:
+	mutex_unlock(&q->mutex);
 	length = -1;
-	goto out;
+	return -1;
 }
 
 static ssize_t qcow_read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
@@ -379,20 +384,22 @@ static ssize_t qcow_write_cluster(struct qcow *q, u64 offset, void *buf, u32 src
 
 	l1t_idx		= get_l1_index(q, offset);
 	if (l1t_idx >= table->table_size)
-		goto error;
+		return -1;
 
 	l2t_idx		= get_l2_index(q, offset);
 	if (l2t_idx >= l2t_sz)
-		goto error;
+		return -1;
 
 	clust_off	= get_cluster_offset(q, offset);
 	if (clust_off >= clust_sz)
-		goto error;
+		return -1;
 
 	len		= clust_sz - clust_off;
 	if (len > src_len)
 		len = src_len;
 
+	mutex_lock(&q->mutex);
+
 	l2t_off		= table->l1_table[l1t_idx] & ~header->oflag_mask;
 	if (l2t_off) {
 		/* read and cache l2 table */
@@ -466,11 +473,14 @@ static ssize_t qcow_write_cluster(struct qcow *q, u64 offset, void *buf, u32 src
 		l2t->table[l2t_idx] = clust_start;
 	}
 
+	mutex_unlock(&q->mutex);
+
 	return len;
 
 free_cache:
 	free(l2t);
 error:
+	mutex_unlock(&q->mutex);
 	return -1;
 }
 
@@ -611,6 +621,7 @@ static struct disk_image *qcow2_probe(int fd, bool readonly)
 	if (!q)
 		goto error;
 
+	mutex_init(&q->mutex);
 	q->fd = fd;
 	q->root = RB_ROOT;
 	INIT_LIST_HEAD(&q->lru_list);
@@ -710,6 +721,7 @@ static struct disk_image *qcow1_probe(int fd, bool readonly)
 	if (!q)
 		goto error;
 
+	mutex_init(&q->mutex);
 	q->fd = fd;
 	q->root = RB_ROOT;
 	INIT_LIST_HEAD(&q->lru_list);
diff --git a/tools/kvm/include/kvm/mutex.h b/tools/kvm/include/kvm/mutex.h
index bd765c4..3286cea 100644
--- a/tools/kvm/include/kvm/mutex.h
+++ b/tools/kvm/include/kvm/mutex.h
@@ -12,6 +12,12 @@
 
 #define DEFINE_MUTEX(mutex) pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER
 
+static inline void mutex_init(pthread_mutex_t *mutex)
+{
+	if (pthread_mutex_init(mutex, NULL) != 0)
+		die("unexpected pthread_mutex_init() failure!");
+}
+
 static inline void mutex_lock(pthread_mutex_t *mutex)
 {
 	if (pthread_mutex_lock(mutex) != 0)
diff --git a/tools/kvm/include/kvm/qcow.h b/tools/kvm/include/kvm/qcow.h
index 12247e0..d44c64a 100644
--- a/tools/kvm/include/kvm/qcow.h
+++ b/tools/kvm/include/kvm/qcow.h
@@ -1,6 +1,8 @@
 #ifndef KVM__QCOW_H
 #define KVM__QCOW_H
 
+#include "kvm/mutex.h"
+
 #include <linux/types.h>
 #include <stdbool.h>
 #include <linux/rbtree.h>
@@ -34,6 +36,7 @@ struct qcow_table {
 };
 
 struct qcow {
+	pthread_mutex_t		mutex;
 	void			*header;
 	struct qcow_table	table;
 	int			fd;
-- 
1.7.0.4


  parent reply	other threads:[~2011-07-09 13:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-09 13:02 [PATCH 0/9] kvm tools, qcow: Improve QCOW performance Pekka Enberg
2011-07-09 13:02 ` [PATCH 1/9] kvm tools, qcow: Rename struct qcow_l2_cache to struct qcow_l2_table Pekka Enberg
2011-07-09 13:02 ` [PATCH 2/9] kvm tools, qcow: Use 'struct qcow_l2_table' instead of untyped array Pekka Enberg
2011-07-09 13:02 ` Pekka Enberg [this message]
2011-07-09 13:02 ` [PATCH 4/9] kvm tools, qcow: Introduce qcow_disk_flush() Pekka Enberg
2011-07-09 13:02 ` [PATCH 5/9] kvm tools, qcow: Delayed L1 table writeout Pekka Enberg
2011-07-09 13:02 ` [PATCH 6/9] kvm tools, qcow: Don't fdatasync() L2 " Pekka Enberg
2011-07-09 13:02 ` [PATCH 7/9] kvm tools, qcow: Use big endian order for L2 table entries Pekka Enberg
2011-07-09 13:02 ` [PATCH 8/9] kvm tools, qcow: Delayed L2 table writeout Pekka Enberg
2011-07-09 13:02 ` [PATCH 9/9] kvm tools, qcow: Flush only dirty L2 tables Pekka Enberg
2011-07-10 17:15 ` [PATCH 0/9] kvm tools, qcow: Improve QCOW performance Ingo Molnar
2011-07-10 18:08   ` Pekka Enberg
2011-07-10 18:17     ` Ingo Molnar
2011-07-10 18:38       ` Pekka Enberg
2011-07-11  9:31     ` Kevin Wolf
2011-07-11  9:41       ` Pekka Enberg
2011-07-11 10:29         ` Kevin Wolf
2011-07-11 10:32           ` Pekka Enberg
2011-07-11 10:36         ` Ingo Molnar
2011-07-11 10:44           ` Pekka Enberg

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=1310216563-17503-4-git-send-email-penberg@kernel.org \
    --to=penberg@kernel.org \
    --cc=asias.hejun@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=mingo@elte.hu \
    --cc=prasadjoshi124@gmail.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.