From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v8 for-xen-4.5 2/2] dpci: Replace tasklet with an softirq (v8) Date: Mon, 27 Oct 2014 17:36:03 +0000 Message-ID: <544E8283.1030907@citrix.com> References: <1413911967-26886-3-git-send-email-konrad.wilk@oracle.com> <5448E82702000078000415B5@mail.emea.novell.com> <20141024015858.GB28850@laptop.dumpdata.com> <544A41970200007800041D63@mail.emea.novell.com> <20141024205544.GA19814@laptop.dumpdata.com> <544E1F590200007800042517@mail.emea.novell.com> <544E2105.1000907@citrix.com> <544E33B902000078000425F0@mail.emea.novell.com> <544E27D6.1040305@citrix.com> <544E397F0200007800042651@mail.emea.novell.com> <20141027170115.GC11893@laptop.dumpdata.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 1XioDF-0002an-Cn for xen-devel@lists.xenproject.org; Mon, 27 Oct 2014 17:36:13 +0000 In-Reply-To: <20141027170115.GC11893@laptop.dumpdata.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: Konrad Rzeszutek Wilk , Jan Beulich Cc: xen-devel@lists.xenproject.org, keir@xen.org, ian.jackson@eu.citrix.com, ian.campbell@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 27/10/14 17:01, Konrad Rzeszutek Wilk wrote: > On Mon, Oct 27, 2014 at 11:24:31AM +0000, Jan Beulich wrote: >>>>> On 27.10.14 at 12:09, wrote: >>> Can it ever be the case that we are waiting for a remote pcpu to run its >>> softirq handler? If so, the time spent looping here could be up to 1 >>> scheduling timeslice in the worst case, and 30ms is a very long time to >>> wait. >> Good point - I think this can be the case. But there seems to be a >> simple counter measure: The first time we get to this point, send an >> event check IPI to the CPU in question (or in the worst case >> broadcast one if the CPU can't be determined in a race free way). > I can either do this using the wrapper: > > if ( pt_pirq_softirq_active(pirq_dpci) ) > { > spin_unlock(&d->event_lock); > if ( pirq_dpci->cpu >= 0 ) > { > cpu_raise_softirq(pirq_dpci->cpu, HVM_DPCI_SOFTIRQ); > pirq_dpci->cpu = -1; > } > cpu_relax(); > goto restart; > > Ought to do it (cpu_raise_softirq will exit out if > the 'pirq_dpci->cpu == smp_processor_id()'). It also has some batching checks > so that we won't do the IPI if we are in the middle of IPI-ing already > an CPU. > > Or just write it out (and bypass some of the checks 'cpu_raise_softirq' > has): > > if ( pt_pirq_softirq_active(pirq_dpci) ) > { > spin_unlock(&d->event_lock); > if ( pirq_dpci->cpu >= 0 && pirq_dpci->cpu != smp_processor_id() ) > { > smp_send_event_check_cpu(pirq_dpci->cpu); > pirq_dpci->cpu = -1; > } > cpu_relax(); > goto restart; > > > Note: > > The 'cpu' is stashed whenever 'raise_softirq_for' has been called. > You need to send at most 1 IPI, or you will be pointlessly spamming the target pcpu. Therefore, a blind goto restart seems ill-advised. The second version doesn't necessarily set HVM_DPCI_SOFTIRQ pending, while the first version suffers a risk of the softirq being caught in a batch. Furthermore, with mwait support, the IPI is elided completely, which is completely wrong in this situation. Therefore, I think you need to manually set the HVM_DPCI_SOFTIRQ bit, then forcibly send the IPI. ~Andrew