Re: [PATCH v3 4/5] evtchn: convert domain event lock to an r/w one

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.12.2020 14:19, Julien Grall wrote:
> On 23/12/2020 12:57, Jan Beulich wrote:
>> On 23.12.2020 12:22, Julien Grall wrote:
>>>> 1) Neither evtchn_status() nor domain_dump_evtchn_info() appear to
>>>> have a real need to acquire the per-domain lock. They could as well
>>>> acquire the per-channel ones. (In the latter case this will then
>>>> also allow inserting the so far missing process_pending_softirqs()
>>>> call; it shouldn't be made with a lock held.)
>>> I agree that evtchn_status() doesn't need to acquire the per-domain
>>> lock. I am not entirely sure about domain_dump_evtchn_info() because
>>> AFAICT the PIRQ tree (used by domain_pirq_to_irq()) is protected with
>>> d->event_lock.
>>
>> It is, but calling it without the lock just to display the IRQ
>> is not a problem afaict.
> 
> How so? Is the radix tree lookup safe against concurrent radix tree 
> insertion/deletion?

Hmm, looks like I was misguided by pirq_field() tolerating NULL
coming back from radix_tree_lookup(). Indeed, if the tree
structure changed, there would be a problem. But this can't be
solved by holding the lock across the entire loop - as said
earlier, the loop body needs to gain a
process_pending_softirqs() invocation, and that needs to happen
with no locks held. The only way I can see us achieving this is
to drop the per-channel lock prior to calling
domain_pirq_to_irq().

Jan