Commit 2c95a329 authored by Asias He's avatar Asias He Committed by Rusty Russell

virtio-blk: Use block layer provided spinlock

Block layer will allocate a spinlock for the queue if the driver does
not provide one in blk_init_queue().

The reason to use the internal spinlock is that blk_cleanup_queue() will
switch to use the internal spinlock in the cleanup code path.

        if (q->queue_lock != &q->__queue_lock)
                q->queue_lock = &q->__queue_lock;

However, processes which are in D state might have taken the driver
provided spinlock, when the processes wake up, they would release the
block provided spinlock.

=====================================
[ BUG: bad unlock balance detected! ]
3.4.0-rc7+ #238 Not tainted
-------------------------------------
fio/3587 is trying to release lock (&(&q->__queue_lock)->rlock) at:
[<ffffffff813274d2>] blk_queue_bio+0x2a2/0x380
but there are no more locks to release!

other info that might help us debug this:
1 lock held by fio/3587:
 #0:  (&(&vblk->lock)->rlock){......}, at:
[<ffffffff8132661a>] get_request_wait+0x19a/0x250

Other drivers use block layer provided spinlock as well, e.g. SCSI.

Switching to the block layer provided spinlock saves a bit of memory and
does not increase lock contention. Performance test shows no real
difference is observed before and after this patch.

Changes in v2: Improve commit log as Michael suggested.

Cc: virtualization@lists.linux-foundation.org
Cc: kvm@vger.kernel.org
Cc: stable@kernel.org
Signed-off-by: default avatarAsias He <asias@redhat.com>
Acked-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent 483001c7
...@@ -21,8 +21,6 @@ struct workqueue_struct *virtblk_wq; ...@@ -21,8 +21,6 @@ struct workqueue_struct *virtblk_wq;
struct virtio_blk struct virtio_blk
{ {
spinlock_t lock;
struct virtio_device *vdev; struct virtio_device *vdev;
struct virtqueue *vq; struct virtqueue *vq;
...@@ -65,7 +63,7 @@ static void blk_done(struct virtqueue *vq) ...@@ -65,7 +63,7 @@ static void blk_done(struct virtqueue *vq)
unsigned int len; unsigned int len;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&vblk->lock, flags); spin_lock_irqsave(vblk->disk->queue->queue_lock, flags);
while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) { while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
int error; int error;
...@@ -99,7 +97,7 @@ static void blk_done(struct virtqueue *vq) ...@@ -99,7 +97,7 @@ static void blk_done(struct virtqueue *vq)
} }
/* In case queue is stopped waiting for more buffers. */ /* In case queue is stopped waiting for more buffers. */
blk_start_queue(vblk->disk->queue); blk_start_queue(vblk->disk->queue);
spin_unlock_irqrestore(&vblk->lock, flags); spin_unlock_irqrestore(vblk->disk->queue->queue_lock, flags);
} }
static bool do_req(struct request_queue *q, struct virtio_blk *vblk, static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
...@@ -431,7 +429,6 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) ...@@ -431,7 +429,6 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
goto out_free_index; goto out_free_index;
} }
spin_lock_init(&vblk->lock);
vblk->vdev = vdev; vblk->vdev = vdev;
vblk->sg_elems = sg_elems; vblk->sg_elems = sg_elems;
sg_init_table(vblk->sg, vblk->sg_elems); sg_init_table(vblk->sg, vblk->sg_elems);
...@@ -456,7 +453,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) ...@@ -456,7 +453,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
goto out_mempool; goto out_mempool;
} }
q = vblk->disk->queue = blk_init_queue(do_virtblk_request, &vblk->lock); q = vblk->disk->queue = blk_init_queue(do_virtblk_request, NULL);
if (!q) { if (!q) {
err = -ENOMEM; err = -ENOMEM;
goto out_put_disk; goto out_put_disk;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment