Re: xen/evtchn and forced threaded irq

[Boris Ostrovsky]

On 2/20/19 9:15 AM, Julien Grall wrote:
> Hi Boris,
>
> Thank you for your answer.
>
> On 20/02/2019 00:02, Boris Ostrovsky wrote:
>> On Tue, Feb 19, 2019 at 05:31:10PM +0000, Julien Grall wrote:
>>> Hi all,
>>>
>>> I have been looking at using Linux RT in Dom0. Once the guest is
>>> started,
>>> the console is ending to have a lot of warning (see trace below).
>>>
>>> After some investigation, this is because the irq handler will now
>>> be threaded.
>>> I can reproduce the same error with the vanilla Linux when passing
>>> the option
>>> 'threadirqs' on the command line (the trace below is from 5.0.0-rc7
>>> that has
>>> not RT support).
>>>
>>> FWIW, the interrupt for port 6 is used to for the guest to
>>> communicate with
>>> xenstore.
>>>
>>> ÂFrom my understanding, this is happening because the interrupt
>>> handler is now
>>> run in a thread. So we can have the following happening.
>>>
>>> ÂÂÂ Interrupt contextÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂÂ Interrupt thread
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |
>>> ÂÂÂ receive interrupt port 6ÂÂÂÂ |
>>> ÂÂÂ clear the evtchn portÂÂÂÂÂÂÂ |
>>> ÂÂÂ set IRQF_RUNTHREADÂÂÂÂÂÂÂÂÂÂÂ |
>>> ÂÂÂ kick interrupt threadÂÂÂÂÂÂÂ |
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ clear IRQF_RUNTHREAD
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ call evtchn_interrupt
>>> ÂÂÂ receive interrupt port 6ÂÂÂÂ |
>>> ÂÂÂ clear the evtchn portÂÂÂÂÂÂÂ |
>>> ÂÂÂ set IRQF_RUNTHREADÂÂÂÂÂÂÂÂÂÂ |
>>> ÂÂÂ kick interrupt threadÂÂÂÂÂÂÂ |
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ disable interrupt port 6
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ evtchn->enabled = false
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ [....]
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ *** Handling the second
>>> interrupt ***
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ clear IRQF_RUNTHREAD
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ call evtchn_interrupt
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂÂ WARN(...)
>>>
>>> I am not entirely sure how to fix this. I have two solutions in mind:
>>>
>>> 1) Prevent the interrupt handler to be threaded. We would also need to
>>> switch from spin_lock to raw_spin_lock as the former may sleep on
>>> RT-Linux.
>>>
>>> 2) Remove the warning
>>
>> I think access to evtchn->enabled is racy so (with or without the
>> warning) we can't use it reliably.
>
> Thinking about it, it would not be the only issue. The ring is sized
> to contain only one instance of the same event. So if you receive
> twice the event, you may overflow the ring.

Hm... That's another argument in favor of "unthreading" the handler.

>
>>
>> Another alternative could be to queue the irq if !evtchn->enabled and
>> handle it in evtchn_write() (which is where irq is supposed to be
>> re-enabled).
> What do you mean by queue? Is it queueing in the ring? 


No, I was thinking about having a new structure for deferred interrupts.

-boris

> If so, wouldn't it be racy as described above?