Re: [RFC] virtio: Support releasing lock during kick

Re: [RFC] virtio: Support releasing lock during kick

[Michael S. Tsirkin]

On Wed, Jun 23, 2010 at 10:24:02PM +0100, Stefan Hajnoczi wrote:
> The virtio block device holds a lock during I/O request processing.
> Kicking the virtqueue while the lock is held results in long lock hold
> times and increases contention for the lock.

As you point out the problem with dropping
and getting this lock in the request function is
that we double the number of atomics here.

How about we teach the block core to call a separate
function with spinlock not held? This way we don't need
to do extra lock/unlock operations, and kick can be
done with lock not held and interrupts enabled.


diff --git a/block/blk-core.c b/block/blk-core.c
index 4ce953f..a8672ec 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -433,6 +433,8 @@ void blk_run_queue(struct request_queue *q)
 	spin_lock_irqsave(q->queue_lock, flags);
 	__blk_run_queue(q);
 	spin_unlock_irqrestore(q->queue_lock, flags);
+	if (q->request_done)
+		q->request_done(q);
 }
 EXPORT_SYMBOL(blk_run_queue);
 

-- 
MST

Re: [RFC] virtio: Support releasing lock during kick

[Christoph Hellwig]

On Sun, Jun 19, 2011 at 10:48:41AM +0300, Michael S. Tsirkin wrote:
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 4ce953f..a8672ec 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -433,6 +433,8 @@ void blk_run_queue(struct request_queue *q)
>  	spin_lock_irqsave(q->queue_lock, flags);
>  	__blk_run_queue(q);
>  	spin_unlock_irqrestore(q->queue_lock, flags);
> +	if (q->request_done)
> +		q->request_done(q);

We have quite a few cases where __blk_run_queue is called directly, and
one more (although not applicable to virtio-blk) that calls ->request_fn
directly.

I think Stefan's way is the way to go for now, releasing and reacquiring
the queue lock once in ->request_fn is much less than the common IDE and
SCSI setups do today.

Eventually ->queue_lock should be split from the driver-internal lock,
and we could do a more efficient calling convention than the one per
request blk_peek_request.  I've started looking into that, but it's
going to take a while.