From: Jens Axboe <axboe@kernel.dk>
To: Shaohua Li <shli@kernel.org>
Cc: Dave Chinner <david@fromorbit.com>, linux-kernel@vger.kernel.org
Subject: Re: [Regression x2, 3.13-git] virtio block mq hang, iostat busted on virtio devices
Date: Tue, 19 Nov 2013 19:02:30 -0700 [thread overview]
Message-ID: <20131120020230.GF4094@kernel.dk> (raw)
In-Reply-To: <20131120015426.GE4094@kernel.dk>
On Tue, Nov 19 2013, Jens Axboe wrote:
> > Looks like a race condition, below works for me, please try.
> >
> >
> > Subject: virtio_blk: fix race condition
> >
> > virtqueue_kick() isn't multi-thread safe.
> >
> > Signed-off-by: Shaohua Li <shli@fusionio.com>
> >
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 588479d..f353959 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -204,10 +204,11 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
> > virtqueue_kick(vblk->vq);
> > return BLK_MQ_RQ_QUEUE_BUSY;
> > }
> > - spin_unlock_irqrestore(&vblk->vq_lock, flags);
> >
> > if (last)
> > virtqueue_kick(vblk->vq);
> > + spin_unlock_irqrestore(&vblk->vq_lock, flags);
> > +
> > return BLK_MQ_RQ_QUEUE_OK;
> > }
>
> Just stumbled on that too. You need one more, btw, for the sg failure
> case:
>
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 588479d58f52..6a680d4de7f1 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -199,15 +199,16 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
>
> spin_lock_irqsave(&vblk->vq_lock, flags);
> if (__virtblk_add_req(vblk->vq, vbr, vbr->sg, num) < 0) {
> + virtqueue_kick(vblk->vq);
> spin_unlock_irqrestore(&vblk->vq_lock, flags);
> blk_mq_stop_hw_queue(hctx);
> - virtqueue_kick(vblk->vq);
> return BLK_MQ_RQ_QUEUE_BUSY;
> }
> - spin_unlock_irqrestore(&vblk->vq_lock, flags);
>
> if (last)
> virtqueue_kick(vblk->vq);
> +
> + spin_unlock_irqrestore(&vblk->vq_lock, flags);
> return BLK_MQ_RQ_QUEUE_OK;
> }
Tested successfully here too. Dave, please give it a go, looks like this
should fix it up for you. Committed here:
http://git.kernel.dk/?p=linux-block.git;a=commit;h=f02b9ac35a47dff745c7637fbc095f01cc03646e
and will send it upstream today, so we can avoid having an -rc1 with a
broken virtio-blk.
--
Jens Axboe
next prev parent reply other threads:[~2013-11-20 2:02 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-19 8:02 [Regression x2, 3.13-git] virtio block mq hang, iostat busted on virtio devices Dave Chinner
2013-11-19 10:36 ` Christoph Hellwig
2013-11-19 16:05 ` Jens Axboe
2013-11-19 16:09 ` Christoph Hellwig
2013-11-19 16:16 ` Jens Axboe
2013-11-19 21:30 ` Dave Chinner
2013-11-19 21:40 ` Jens Axboe
2013-11-19 20:15 ` Jens Axboe
2013-11-19 21:20 ` Jens Axboe
2013-11-19 21:34 ` Dave Chinner
2013-11-19 21:43 ` Jens Axboe
2013-11-19 22:42 ` Jens Axboe
2013-11-19 22:51 ` Jens Axboe
2013-11-19 23:23 ` Dave Chinner
2013-11-19 23:59 ` Jens Axboe
2013-11-20 0:08 ` Jens Axboe
2013-11-20 1:44 ` Shaohua Li
2013-11-20 1:54 ` Jens Axboe
2013-11-20 2:02 ` Jens Axboe [this message]
2013-11-20 2:53 ` Dave Chinner
2013-11-20 3:12 ` Jens Axboe
2013-11-20 8:07 ` Christoph Hellwig
2013-11-20 16:21 ` Jens Axboe
2013-11-20 8:04 ` Christoph Hellwig
2013-11-20 16:20 ` Jens Axboe
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=20131120020230.GF4094@kernel.dk \
--to=axboe@kernel.dk \
--cc=david@fromorbit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=shli@kernel.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 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.