Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: xen/evtchn and forced threaded irq
To:     Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc:     Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        xen-devel <xen-devel@lists.xenproject.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Dave P Martin <dave.martin@arm.com>
References: <5e256d9a-572c-e01e-7706-407f99245b00@arm.com>
 <20190220000209.GA4091@localhost.localdomain>
 <a872d480-9f1b-6cd7-e507-ac4fcdf705af@arm.com>
 <21214d47-9c68-e6bf-007a-4047cc2b02f9@oracle.com>
From:   Julien Grall <julien.grall@arm.com>
Message-ID: <8f7445d7-fa50-f3e9-44f5-cc2aebd020f4@arm.com>
Date:   Wed, 20 Feb 2019 18:05:22 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <21214d47-9c68-e6bf-007a-4047cc2b02f9@oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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