Re: xen/evtchn and forced threaded irq

[Boris Ostrovsky]

On 2/20/19 1:05 PM, Julien Grall wrote:
> Hi,
>
> On 20/02/2019 17:07, Boris Ostrovsky wrote:
>> 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.
>
> I first thought it would be possible to unthread it. However,
> wake_up_interruptible is using a spin_lock. On RT spin_lock can sleep,
> so this cannot be used in an interrupt context.
>
> So I think "unthreading" the handler is not an option here.

That sounds like a different problem. I.e. there are two issues:
* threaded interrupts don't work properly (races, ring overflow)
* evtchn_interrupt() (threaded or not) has spin_lock(), which is not
going to work for RT

The first can be fixed by using non-threaded handlers.

>
>>
>>>
>>>>
>>>> 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.
>
> Hmmm, I am not entirely sure what would be the structure here. Could
> you expand your thinking?

Some sort of a FIFO that stores {irq, data} tuple. It could obviously be
implemented as a ring but not necessarily as Xen shared ring (if that's
what you were referring to).

-boris