qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes


From: Anthony Liguori
Subject: Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes
Date: Mon, 20 Sep 2010 10:48:59 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100826 Lightning/1.0b1 Thunderbird/3.0.7

On 09/20/2010 10:33 AM, Kevin Wolf wrote:
Am 20.09.2010 16:56, schrieb Anthony Liguori:
+void blkqueue_flush(BlockQueue *bq)
+{
+    qemu_mutex_lock(&bq->flush_lock);
+
+    /* Process any left over requests */
+    while (QTAILQ_FIRST(&bq->queue)) {
+        blkqueue_process_request(bq);
+    }
+
+    qemu_mutex_unlock(&bq->flush_lock);
+}
+
+static void *blkqueue_thread(void *_bq)
+{
+    BlockQueue *bq = _bq;
+#ifndef RUN_TESTS
+    BlockQueueRequest *req;
+#endif
+
+    qemu_mutex_lock(&bq->flush_lock);
+    while (!bq->thread_done) {
+        barrier();
A barrier shouldn't be needed here.
It was needed when I started with an empty thread because gcc would
"optimize" while(!bq->thread_done) into an endless loop. I guess there
is enough code added now that gcc won't try to be clever any more, so I
can remove that.

The qemu_cond_wait() will act as a read barrier.

A less invasive way of doing this (assuming we're okay with it from a
correctness perspective) is to make use of qemu_aio_wait() as a
replacement for qemu_mutex_lock() and shift the pread/pwrite calls to
bdrv_aio_write/bdrv_aio_read.

IOW, blkqueue_pwrite stages a request via bdrv_aio_write().
blkqueue_pread() either returns a cached read or it does a
bdrv_pread().  The blkqueue_flush() call will then do qemu_aio_wait() to
wait for all pending I/Os to complete.
I was actually considering that, but it would have been a bit more
coding to keep track of another queue of in-flight requests, juggling
with some more AIOCBs and implementing an emulation for the missing
bdrv_aio_pwrite. Nothing really dramatic, it just was easier to start
this way.

bdrv_aio_pwritev is definitely useful in other places so it's worth adding.

If we come to the conclusion that bdrv_aio_write is the way to go and
it's worth the work, I'm fine with changing it.

Adding locking to allow bdrv_pwrite/bdrv_pread to be safely called outside of qemu_mutex is going to carry an awful lot of complexity since we can do things like layer qcow2 on top of NBD. That means bdrv_pread() may be repeatedly interacting with the main loop which means that there's no simple place to start.

I'm not fundamentally opposed to using threads for concurrency. I think it's going to get super complicated though to do it here.

Regards,

Anthony Liguori

Kevin




reply via email to

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