Re: xen/evtchn and forced threaded irq

[Julien Grall]

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.

> > > 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?

Cheers,

--
Julien Grall