Re: [PATCH 1/2] vhost: allow multiple workers threads

On 02/20/2012 01:27 PM, Michael S. Tsirkin wrote:

On Mon, Feb 20, 2012 at 09:50:37AM -0600, Tom Lendacky wrote:

quoted

"Michael S. Tsirkin"[off-list ref]  wrote on 02/19/2012 08:41:45 AM:

quoted

From: "Michael S. Tsirkin"<mst@redhat.com>
To: Anthony Liguori/Austin/IBM@IBMUS
Cc: netdev@vger.kernel.org, Tom Lendacky/Austin/IBM@IBMUS, Cristian
Viana[off-list ref]
Date: 02/19/2012 08:42 AM
Subject: Re: [PATCH 1/2] vhost: allow multiple workers threads

On Fri, Feb 17, 2012 at 05:02:05PM -0600, Anthony Liguori wrote:

quoted

This patch allows vhost to have multiple worker threads for devices

such as

quoted

quoted

virtio-net which may have multiple virtqueues.

Since virtqueues are a lockless ring queue, in an ideal world data is

being

quoted

quoted

produced by the producer as fast as data is being consumed by the

consumer.

quoted

quoted

These loops will continue to consume data until none is left.

vhost currently multiplexes the consumer side of the queue on a

single thread

quoted

by attempting to read from the queue until everything is read or it

cannot

quoted

quoted

process anymore.  This means that activity on one queue may stall

another queue.

There's actually an attempt to address this: look up
VHOST_NET_WEIGHT in the code. I take it, this isn't effective?

quoted

This is exacerbated when using any form of polling to read from

the queues (as

quoted

we'll introduce in the next patch).  By spawning a thread per-

virtqueue, this

quoted

is addressed.

The only problem with this patch right now is how the wake up of

the threads is

quoted

done.  It's essentially a broadcast and we have seen lock contention as

a

quoted

quoted

result.

On which lock?

The mutex lock in the vhost_virtqueue struct.  This really shows up when
running with patch 2/2 and increasing the spin_threshold. Both threads wake
up and try to acquire the mutex.  As the spin_threshold increases you end
up
with one of the threads getting blocked for a longer and longer time and
unable to do any RX processing that might be needed.

Tom

Weird, I had the impression each thread handles one vq.
Isn't this the design?

Not the way the code is structured today.  There is a single consumer/producer 
work queue and either the vq notification or other actions may get placed on it.

It would be possible to do three threads, one for background tasks and then one 
for each queue with a more invasive refactoring.

But I assumed that the reason the code was structured this was originally was 
because you saw some value in having a single producer/consumer queue for 
everything...

Regards,

Anthony Liguori