linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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 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).