From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: RUNSTATE_runnable delta time for idle_domain accounted to HVM guest. Date: Wed, 7 May 2014 09:33:08 -0400 Message-ID: <20140507133308.GD9190@phenom.dumpdata.com> References: <20140423212824.GB12560@phenom.dumpdata.com> <535F6DF7.9080704@eu.citrix.com> <20140429124206.GB10696@phenom.dumpdata.com> <20140506173627.GA6942@phenom.dumpdata.com> <536A05E4020000780000FB39@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wi1yK-0003GQ-Mk for xen-devel@lists.xenproject.org; Wed, 07 May 2014 13:33:20 +0000 Content-Disposition: inline In-Reply-To: <536A05E4020000780000FB39@mail.emea.novell.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: Jan Beulich Cc: George Dunlap , xen-devel@lists.xenproject.org, dario.faggioli@citrix.com, keir.xen@gmail.com List-Id: xen-devel@lists.xenproject.org On Wed, May 07, 2014 at 09:07:32AM +0100, Jan Beulich wrote: > >>> On 06.05.14 at 19:36, wrote: > > The reason we schedule a tasklet instead of continuing with an > > 'vcpu_kick' is not yet known to me. This commit added the mechanism > > to do it via the tasklet: I should also add that a bit more digging and I realized that the reason we have so many of the tasklets running (the guests couldn't be that insane to schedule so many alarms for one-shot timers!) is that we do PCI passthrough. And that uses the 'hvm_dirq_assist' tasklet (hvm_do_IRQ_dpci). As in we serialize passthrough interrupts for all of the guests via this tasklet lock. And in this particular setups, the other sockets are busy doing I/O over PCI passthrough devices. Hence the lock is taken quite frequently - over all off the sockets. > > > > commit a5db2986d47fafc5e62f992616f057bfa43015d9 > > Author: Keir Fraser > > Date: Fri May 8 11:50:12 2009 +0100 > > > > x86 hvm: hvm_set_callback_irq_level() must not be called in IRQ > > context or with IRQs disabled. Ensure this by deferring to tasklet > > (softirq) context if required. > > > > Signed-off-by: Keir Fraser > > > > But I am not sure why: > > > > a). 'must not be called in IRQ context or with IRQs disabled' is > > important - I haven't dug in the code yet to understand the > > crucial reasons for - is there a known issue about this? > > Because of its use of spin_lock(), which would have the potential > for a deadlock if the function was called in the wrong context. The > apparent alternative would be to make all users of > d->arch.hvm_domain.irq_lock use the IRQ-safe variant; I didn't > check whether that would in turn cause new problems. Thanks for the pointer. > > > b). Why do we have a per-cpu tasklet lists, but any manipulation of the > > items of them are protected by a global lock. Looking at the code in > > Linux and Xen the major difference is that Xen can schedule on specific CPUs > > (or even the tasklet can schedule itself on another CPU). > > tasklet_kill() and migrate_tasklets_from_cpu() at the very least > would need very careful modification if you were to replace the > global lock. That was my feeling as well. > > > I can see the need for the tasklets being on different CPUs for > > the microcode, and I am digging through the other ones to get > > a feel for it - but has anybody thought about improving this > > code? Has there been any suggestions/ideas tossed around in the > > past (the mailing list didn't help or my Google-fun sucks). > > I'm not aware of any such, quite likely because no-one so far > spotted a problem with the current implementation. Yeey! First one to discover! > > Jan >