Re: [PATCH] evtchn/Flask: pre-allocate node on send path

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 25/09/2020 13:21, Jan Beulich wrote:
> On 25.09.2020 12:34, Julien Grall wrote:
> > On 24/09/2020 11:53, Jan Beulich wrote:
> > > xmalloc() & Co may not be called with IRQs off, or else check_lock()
> > > will have its assertion trigger about locks getting acquired
> > > inconsistently. Re-arranging the locking in evtchn_send() doesn't seem
> > > very reasonable, especially since the per-channel lock was introduced to
> > > avoid acquiring the per-domain event lock on the send paths. Issue a
> > > second call to xsm_evtchn_send() instead, before acquiring the lock, to
> > > give XSM / Flask a chance to pre-allocate whatever it may need.
> > This is the sort of fall-out I was expecting when we decide to turn off
> > the interrupts for big chunk of code. I couldn't find any at the time
> > though...
> >
> > Can you remind which caller of send_guest{global, vcpu}_virq() will call
> > them with interrupts off?
> I don't recall which one of the two it was that I hit; we wanted
> both to use the lock anyway. send_guest_pirq() very clearly also
> gets called with IRQs off.
>
> > Would it be possible to consider deferring the call to a softirq
> > taslket? If so, this would allow us to turn the interrupts again.
> Of course this is in principle possible; the question is how
> involved this is going to get. 
> However, on x86 oprofile's call to
> send_guest_vcpu_virq() can't easily be replaced - it's dangerous
> enough already that in involves locks in NMI context. I don't
> fancy seeing it use more commonly used ones.
Fair enough. I would still like to consider a way where we could avoid 
to hack xsm_* because we have the interrupts disabled.
AFAICT, we don't need send_guest_global_virq() and evtchn_send() to be 
mutually exclusive. Is that correct?
So how about splitting the lock in two? One would be used when the 
interrupts have to be disabled while the other would be used when we can 
keep interrupts enabled.
The two locks would have to be taken when the event channel needs to be 
modified.
Cheers,

--
Julien Grall