Re: Ping: [PATCH v5 0/6] evtchn: (not so) recent XSAs follow-on

[Julien Grall <julien@xen.org>]

On 21/04/2021 16:23, Jan Beulich wrote:
> On 27.01.2021 09:13, Jan Beulich wrote:
>> These are grouped into a series largely because of their origin,
>> not so much because there are (heavy) dependencies among them.
>> The main change from v4 is the dropping of the two patches trying
>> to do away with the double event lock acquires in interdomain
>> channel handling. See also the individual patches.
>>
>> 1: use per-channel lock where possible
>> 2: convert domain event lock to an r/w one
>> 3: slightly defer lock acquire where possible
>> 4: add helper for port_is_valid() + evtchn_from_port()
>> 5: type adjustments
>> 6: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq()
> 
> Only patch 4 here has got an ack so far - may I ask for clear feedback
> as to at least some of these being acceptable (I can see the last one
> being controversial, and if this was the only one left I probably
> wouldn't even ping, despite thinking that it helps reduce unecessary
> overhead).

I left feedback for the series one the previous version (see [1]). It 
would have been nice is if it was mentionned somewhere as this is still 
unresolved.

The locking in the event channel is already quite fragile (see recent 
XSAs, follow-up bugs...). Even if the pattern is used somewhere (as you 
suggested), I don't think it is good idea to pertain it.

To be clear, I am not saying I don't care about performance. Instead I 
am trying to find a trade off between code maintenability and 
performance. So far, I didn't see any data justifying that the extra 
performance is worth the risk of making a code more fragile.

Cheers,

[1] <3c393170-09f9-6d31-c227-b599f8769e35@xen.org>

> 
> Jan
> 

-- 
Julien Grall