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

[Julien Grall <julien@xxxxxxx>]

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@xxxxxxx>

> Jan
--
Julien Grall