Re: [PATCH v2 net-next] virtio: Fix affinity for #VCPUs != #queue pairs

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Feb 15, 2017 at 10:27:37AM -0800, Benjamin Serebrin wrote:
> On Wed, Feb 15, 2017 at 9:42 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> >
> > > For pure network load, assigning each txqueue IRQ exclusively
> > > to one of the cores that generates traffic on that queue is the
> > > optimal layout in terms of load spreading. Irqbalance does
> > > not have the XPS information to make this optimal decision.
> >
> > Try to add hints for it?
> >
> >
> > > Overall system load affects this calculation both in the case of 1:1
> > > mapping uneven queue distribution. In both cases, irqbalance
> > > is hopefully smart enough to migrate other non-pinned IRQs to
> > > cpus with lower overall load.
> >
> > Not if everyone starts inserting hacks like this one in code.
> 
> 
> It seems to me that the default behavior is equally "random" - why would we want
> XPS striped across the cores the way it's done today?

Right. But userspace knows it's random at least. If kernel supplies
affinity e.g. the way your patch does, userspace ATM accepts this as a
gospel.

> What we're trying to do here is avoid the surprise cliff that guests will
> hit when queue count is limited to less than VCPU count.

And presumably the high VCPU count is there because people
actually run some tasks on all these extra VCPUs.
so I'm not sure I buy the "pure networking workload" argument.

> That will
> happen because
> we limit queue pair count to 32.  I'll happily push further complexity
> to user mode.


Why do you do this BTW? I note that this will interfere with e.g. XPS
which for better or worse wants its own set of TC queues.

> If this won't fly, we can leave all of this behavior in user code.
> Michael, would
> you prefer that I abandon this patch?

I think it's either that or find a way that does not interfere with what
existing userspace has been doing. That's likely to involve core changes
outside of virtio.

> > That's another problem with this patch. If you care about hyperthreads
> > you want an API to probe for that.
> 
> 
> It's something of a happy accident that hyperthreads line up that way.
> Keeping the topology knowledge out of the patch and into user space seems
> cleaner, would you agree?
> 
> Thanks!
> Ben

Well kernel does have topology knowledge already but I have no
issue with keeping the logic all in userspace.

-- 
MST