From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [RFC PATCH v1] Replace tasklets with per-cpu implementation. Date: Tue, 2 Sep 2014 16:10:01 -0400 Message-ID: <20140902201001.GD16113@laptop.dumpdata.com> References: <5400E95E.8080808@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XOuP4-0000jp-Sv for xen-devel@lists.xenproject.org; Tue, 02 Sep 2014 20:10:11 +0000 Content-Disposition: inline In-Reply-To: <5400E95E.8080808@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Arianna Avanzini Cc: xen-devel@lists.xenproject.org, Keir Fraser , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Fri, Aug 29, 2014 at 10:58:06PM +0200, Arianna Avanzini wrote: > > Hey, > > > > With the Xen 4.5 feature freeze being right on the doorsteps I am not > > expecting this to go in as: > > 1) It touches core code, > > 2) It has never been tested on ARM, > > Sorry if I intrude - for what it's worth, the patchset works on my setup. I am > running Xen from the development repository, plus this patchset, with a Linux > 3.15 dom0 (linux-sunxi) on a cubieboard2. Woot! Thank you! > > > > 3) It is RFC for right now. > > > > With those expectations out of the way, I am submitting for review > > an over-haul of the tasklet code. We had found one large machines > > with a small size of guests (12) that the steal time for idle > > guests was excessively high. Further debugging revealed that the > > global tasklet lock was taken across all sockets at an excessively > > high rate. To the point that 1/10th of a guest idle time was > > taken (and actually accounted for as in RUNNING state!). > > > > The ideal situation to reproduce this behavior is: > > 1). Allocate a twelve guests with one to four SR-IOV VFs. > > 2). Have half of them (six) heavily use the SR-IOV VFs devices. > > 3). Monitor the rest (which are in idle) and despair. > > > > As I discovered under the hood, we have two tasklets that are > > scheduled and executed quite often - the VIRQ_TIMER one: > > aassert_evtchn_irq_taskle, and the one in charge of injecting > > an PCI interrupt in the guest: hvm_do_IRQ_dpci. > > > > The 'hvm_do_IRQ_dpci' is the on that is most often scheduled > > and run. The performance bottleneck comes from the fact that > > we take the same spinlock three times: tasklet_schedule, > > when we are about to execute the tasklet, and when we are > > done executing the tasklet. > > > > This patchset throws away the global list and lock for all > > tasklets. Instead there are two per-cpu lists: one for > > softirq, and one run when scheduler decides it. There is also > > an global list and lock when we have cross-CPU tasklet scheduling > > - which thankfully rarely happens (microcode update and > > hypercall continuation). > > > > The insertion and removal from the list is done by disabling > > interrupts - which are short bursts of time. The behavior > > of the code to only execute one tasklet per iteration is > > also preserved (the Linux code would run through all > > of its tasklets). > > > > The performance benefit of this patch were astounding and > > removed the issues we saw. It also decreased the IRQ > > latency of delievering an interrupt to a guest. > > > > In terms of the patchset I choose an path in which: > > 0) The first patch fixes the performance bug we saw and it > > was easy to backport. > > 1) It is bisectable. > > 2) If something breaks it should be fairly easy to figure > > out which patch broke it. > > 3) It is spit up in a bit weird fashion with scaffolding code > > was added to keep it ticking (as at some point we have > > the old and the new implementation existing and used). > > And then later on removed. This is how Greg KH added > > kref and kobjects long time ago in the kernel and it had > > worked - so I figured I would borrow from this workflow. > > > > I would appreciate feedback from the maintainers if they > > would like this to be organized better. > > > > xen/common/tasklet.c | 305 +++++++++++++++++++++++++++++++++------------- > > xen/include/xen/tasklet.h | 52 +++++++- > > 2 files changed, 271 insertions(+), 86 deletions(-) > > > > Konrad Rzeszutek Wilk (5): > > tasklet: Introduce per-cpu tasklet for softirq. > > tasklet: Add cross CPU feeding of per-cpu tasklets. > > tasklet: Remove the old-softirq implementation. > > tasklet: Introduce per-cpu tasklet for schedule tasklet. > > tasklet: Remove the scaffolding.