Messages in this thread | | | Subject | Re: [PATCH] x86/xen: remove unneeded preempt_disable() from xen_irq_enable() | From | Juergen Gross <> | Date | Tue, 21 Sep 2021 09:58:28 +0200 |
| |
On 21.09.21 09:53, Jan Beulich wrote: > On 21.09.2021 09:02, Juergen Gross wrote: >> --- a/arch/x86/xen/irq.c >> +++ b/arch/x86/xen/irq.c >> @@ -57,24 +57,20 @@ asmlinkage __visible void xen_irq_enable(void) >> { >> struct vcpu_info *vcpu; >> >> - /* >> - * We may be preempted as soon as vcpu->evtchn_upcall_mask is >> - * cleared, so disable preemption to ensure we check for >> - * events on the VCPU we are still running on. >> - */ >> - preempt_disable(); >> - >> vcpu = this_cpu_read(xen_vcpu); >> vcpu->evtchn_upcall_mask = 0; >> >> - /* Doesn't matter if we get preempted here, because any >> - pending event will get dealt with anyway. */ >> + /* >> + * Now preemption could happen, but this is only possible if an event >> + * was handled, so missing an event due to preemption is not >> + * possible at all. >> + * The worst possible case is to be preempted and then check events >> + * pending on the old vcpu, but this is not problematic. >> + */ > > I agree this isn't problematic from a functional perspective, but ... > >> barrier(); /* unmask then check (avoid races) */ >> if (unlikely(vcpu->evtchn_upcall_pending)) >> xen_force_evtchn_callback(); > > ... is a stray call here cheaper than ... > >> - >> - preempt_enable(); > > ... the preempt_{dis,en}able() pair?
The question is if a stray call in case of preemption (very unlikely) is cheaper than the preempt_{dis|en}able() pair on each IRQ enabling.
I'm quite sure removing the preempt_*() calls will be a net benefit.
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |