Re: [Xen-devel] [PATCH v8 for-xen-4.5 2/2] dpci: Replace tasklet with an softirq (v8)

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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, <andrew.cooper3@xxxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel