[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes |
Date: |
Mon, 20 Sep 2010 17:33:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100907 Fedora/3.0.7-1.fc12 Thunderbird/3.0.7 |
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.
>>> +#ifndef RUN_TESTS
>>> + req = QTAILQ_FIRST(&bq->queue);
>>> +
>>> + /* Don't process barriers, we only do that on flushes */
>>> + if (req&& (req->type != REQ_TYPE_BARRIER ||
>>> bq->queue_size> 42)) {
>>> + blkqueue_process_request(bq);
>>> + } else {
>>> + qemu_cond_wait(&bq->cond,&bq->flush_lock);
>>> + }
>
>
> The normal pattern for this is:
>
> while (!condition) {
> qemu_cond_wait(&cond, &lock);
> }
> process_request()
>
> It's generally best not to deviate from this pattern in terms of code
> readability.
Hm, yes, I think you're right. The code used to be a bit more involved
here initially and it seems that I missed the last obvious piece of
simplification.
> 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.
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.
Kevin
- [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Kevin Wolf, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Anthony Liguori, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Kevin Wolf, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Anthony Liguori, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Kevin Wolf, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Anthony Liguori, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Anthony Liguori, 2010/09/20
- Re: [Qemu-devel] [RFC] block-queue: Delay and batch metadata writes, Avi Kivity, 2010/09/20