Re: [RFC PATCH v1] Replace tasklets with per-cpu implementation.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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.