From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH v2 net-next] virtio: Fix affinity for #VCPUs != #queue pairs Date: Wed, 15 Feb 2017 17:23:25 +0200 Message-ID: <20170214224033-mutt-send-email-mst@kernel.org> References: <20170207181506.36668-1-serebrin@google.com> <20170208205353-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mx1.redhat.com ([209.132.183.28]:55949 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750810AbdBOPX2 (ORCPT ); Wed, 15 Feb 2017 10:23:28 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Benjamin Serebrin Cc: Christian Borntraeger , netdev@vger.kernel.org, Jason Wang , David Miller , Willem de Bruijn , Venkatesh Srinivas , "Jon Olson (Google Drive)" , willemdebruijn.kernel@gmail.com, Rick Jones , James Mattson , linux-s390 , "linux-arch@vger.kernel.org" On Tue, Feb 14, 2017 at 11:17:41AM -0800, Benjamin Serebrin wrote: > On Wed, Feb 8, 2017 at 11:37 AM, Michael S. Tsirkin wrote: > > > IIRC irqbalance will bail out and avoid touching affinity > > if you set affinity from driver. Breaking that's not nice. > > Pls correct me if I'm wrong. > > > I believe you're right that irqbalance will leave the affinity alone. > > Irqbalance has had changes that may or may not be in the versions bundled with > various guests, and I don't have a definitive cross-correlation of irqbalance > version to guest version. But in the existing code, the driver does > set affinity for #VCPUs==#queues, so that's been happening anyway. Right but only for the case where we are very sure we are doing the right thing, so we don't need any help from irqbalance. > The (original) intention of this patch was to extend the existing behavior > to the case where we limit queue counts, to avoid the surprising discontinuity > when #VCPU != #queues. > > It's not obvious that it's wrong to cause irqbalance to leave these > queues alone: Generally you want the interrupt to come to the core that > caused the work, to have cache locality and avoid lock contention. > Doing fancier things is outside the scope of this patch. Doing fancier things like trying to balance the load would be in scope for irqbalance so I think you need to find a way to supply default affinity without disabling irqbalance. > > Doesn't look like this will handle the case of num cpus < num queues well. > > I believe it's correct. The first #VCPUs queues will have one bit set in their > xps mask, and the remaining queues have no bits set. That means each VCPU uses > its own assigned TX queue (and the TX interrupt comes back to that VCPU). > > Thanks again for the review! > Ben