Re: [PATCH v2] virtio_blk: unlock vblk->lock during kick

[Asias He <asias@redhat.com>]

On 06/04/2012 09:57 AM, Rusty Russell wrote:
> On Wed, 30 May 2012 15:39:05 +0200, Christian Borntraeger<borntraeger@de.ibm.com>  wrote:
>> On 30/05/12 15:19, Stefan Hajnoczi wrote:
>>> Holding the vblk->lock across kick causes poor scalability in SMP
>>> guests.  If one CPU is doing virtqueue kick and another CPU touches the
>>> vblk->lock it will have to spin until virtqueue kick completes.
>>>
>>> This patch reduces system% CPU utilization in SMP guests that are
>>> running multithreaded I/O-bound workloads.  The improvements are small
>>> but show as iops and SMP are increased.
>>
>> Funny, recently I got a bug report regarding spinlock lockup
>> (see http://lkml.indiana.edu/hypermail/linux/kernel/1205.3/02201.html)
>> Turned out that blk_done was called on many guest cpus while the guest
>> was heavily paging on one virtio block device. (and the guest had much
>> more cpus than the host)
>> This patch will probably reduce the pressure for those cases as well.
>> we can then finish requests if somebody else is doing the kick.
>>
>> IIRC there were some other approaches to address this lock holding during
>> kick but this looks like the less intrusive one.
>>
>>> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
>> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com>
>
> Unfortunately, this conflicts with Asias He's deadlock fix, which has
> us just using the block-layer-supplied spinlock.
>
> If we drop the lock around the kick as you suggest, we're playing with
> fire.  All the virtio backends have an atomic notify, so they're OK,
> and the block layer *looks* safe at a glance, but there's no assurances.

Why are we playing with fire if we drop the lock around the kick?
Which one do you think is un-safe, calling virtqueue_notify() with lock 
dropped or dropping the lock in q->request_fn()?

> It seems like a workaround to the fact that we don't have hcall-backed
> spinlocks like Xen, or that our virtio device is too laggy.
>
> Cheers,
> Rusty.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Asias