[PATCH] xen/events: remove event handling recursion detection

[PATCH] xen/events: remove event handling recursion detection

[Juergen Gross]

__xen_evtchn_do_upcall() contains guards against being called
recursively. This mechanism was introduced in the early pvops times
(kernel 2.6.26) when there were still Xen versions around not honoring
disabled interrupts for sending events to pv guests.

This was changed in Xen 3.0, which is much older than any Xen version
supported by the kernel, so the recursion detection can be removed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/events/events_base.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 6c8843968a52..33212c494afd 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1213,31 +1213,21 @@ void xen_send_IPI_one(unsigned int cpu, enum ipi_vector vector)
 	notify_remote_via_irq(irq);
 }
 
-static DEFINE_PER_CPU(unsigned, xed_nesting_count);
-
 static void __xen_evtchn_do_upcall(void)
 {
 	struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
-	int cpu = get_cpu();
-	unsigned count;
+	int cpu = smp_processor_id();
 
 	do {
 		vcpu_info->evtchn_upcall_pending = 0;
 
-		if (__this_cpu_inc_return(xed_nesting_count) - 1)
-			goto out;
-
 		xen_evtchn_handle_events(cpu);
 
 		BUG_ON(!irqs_disabled());
 
-		count = __this_cpu_read(xed_nesting_count);
-		__this_cpu_write(xed_nesting_count, 0);
-	} while (count != 1 || vcpu_info->evtchn_upcall_pending);
-
-out:
+		rmb(); /* Hypervisor can set upcall pending. */
 
-	put_cpu();
+	} while (vcpu_info->evtchn_upcall_pending);
 }
 
 void xen_evtchn_do_upcall(struct pt_regs *regs)
-- 
2.16.4

Re: [Xen-devel] [PATCH] xen/events: remove event handling recursion detection

[Jan Beulich]

On 04.11.2019 14:58, Juergen Gross wrote:
> __xen_evtchn_do_upcall() contains guards against being called
> recursively. This mechanism was introduced in the early pvops times
> (kernel 2.6.26) when there were still Xen versions around not honoring
> disabled interrupts for sending events to pv guests.
> 
> This was changed in Xen 3.0, which is much older than any Xen version
> supported by the kernel, so the recursion detection can be removed.

Would you mind pointing out which exact change(s) this was(were)?
It had always been my understanding that the recursion detection
was mainly to guard against drivers re-enabling interrupts
transiently in their handlers (which in turn may no longer be an
issue in modern Linux kernels).

Jan

Re: [Xen-devel] [PATCH] xen/events: remove event handling recursion detection

[Jürgen Groß]

On 04.11.19 15:35, Jan Beulich wrote:
> On 04.11.2019 14:58, Juergen Gross wrote:
>> __xen_evtchn_do_upcall() contains guards against being called
>> recursively. This mechanism was introduced in the early pvops times
>> (kernel 2.6.26) when there were still Xen versions around not honoring
>> disabled interrupts for sending events to pv guests.
>>
>> This was changed in Xen 3.0, which is much older than any Xen version
>> supported by the kernel, so the recursion detection can be removed.
> 
> Would you mind pointing out which exact change(s) this was(were)?

Linux kernel: 229664bee6126e01f8662976a5fe2e79813b77c8
Xen: d8263e8dbaf5ef1445bee0662143a0fcb6d43466

> It had always been my understanding that the recursion detection
> was mainly to guard against drivers re-enabling interrupts
> transiently in their handlers (which in turn may no longer be an
> issue in modern Linux kernels).

This would have been doable with a simple bool. The more complex
xchg based logic was IMO for recursion detection at any point.


Juergen

Re: [Xen-devel] [PATCH] xen/events: remove event handling recursion detection

[Jan Beulich]

On 04.11.2019 16:09, Jürgen Groß wrote:
> On 04.11.19 15:35, Jan Beulich wrote:
>> On 04.11.2019 14:58, Juergen Gross wrote:
>>> __xen_evtchn_do_upcall() contains guards against being called
>>> recursively. This mechanism was introduced in the early pvops times
>>> (kernel 2.6.26) when there were still Xen versions around not honoring
>>> disabled interrupts for sending events to pv guests.
>>>
>>> This was changed in Xen 3.0, which is much older than any Xen version
>>> supported by the kernel, so the recursion detection can be removed.
>>
>> Would you mind pointing out which exact change(s) this was(were)?
> 
> Linux kernel: 229664bee6126e01f8662976a5fe2e79813b77c8
> Xen: d8263e8dbaf5ef1445bee0662143a0fcb6d43466

Are you sure about the latter, touching only header files underneath
xen/, and there mostly public interface ones?

>> It had always been my understanding that the recursion detection
>> was mainly to guard against drivers re-enabling interrupts
>> transiently in their handlers (which in turn may no longer be an
>> issue in modern Linux kernels).
> 
> This would have been doable with a simple bool. The more complex
> xchg based logic was IMO for recursion detection at any point.

Well, the respective XenoLinux c/s 13098 has no mention of this, i.e.
it simply leaves open what the actual reason was:

"[LINUX] Disallow nested event delivery.

 This eliminates the risk of overflowing the kernel stack and is a
 reasonable policy given that we have no concept of priorities among
 event sources."

Jan

Re: [Xen-devel] [PATCH] xen/events: remove event handling recursion detection

[Jürgen Groß]

On 04.11.19 16:19, Jan Beulich wrote:
> On 04.11.2019 16:09, Jürgen Groß wrote:
>> On 04.11.19 15:35, Jan Beulich wrote:
>>> On 04.11.2019 14:58, Juergen Gross wrote:
>>>> __xen_evtchn_do_upcall() contains guards against being called
>>>> recursively. This mechanism was introduced in the early pvops times
>>>> (kernel 2.6.26) when there were still Xen versions around not honoring
>>>> disabled interrupts for sending events to pv guests.
>>>>
>>>> This was changed in Xen 3.0, which is much older than any Xen version
>>>> supported by the kernel, so the recursion detection can be removed.
>>>
>>> Would you mind pointing out which exact change(s) this was(were)?
>>
>> Linux kernel: 229664bee6126e01f8662976a5fe2e79813b77c8
>> Xen: d8263e8dbaf5ef1445bee0662143a0fcb6d43466
> 
> Are you sure about the latter, touching only header files underneath
> xen/, and there mostly public interface ones?

No, you are right, this was a false interpretation of mine.

> 
>>> It had always been my understanding that the recursion detection
>>> was mainly to guard against drivers re-enabling interrupts
>>> transiently in their handlers (which in turn may no longer be an
>>> issue in modern Linux kernels).
>>
>> This would have been doable with a simple bool. The more complex
>> xchg based logic was IMO for recursion detection at any point.
> 
> Well, the respective XenoLinux c/s 13098 has no mention of this, i.e.
> it simply leaves open what the actual reason was:
> 
> "[LINUX] Disallow nested event delivery.
> 
>   This eliminates the risk of overflowing the kernel stack and is a
>   reasonable policy given that we have no concept of priorities among
>   event sources."

For XenoLinux it makes at least a little bit sense, as interrupts
were enabled during calls of some handlers AFAIK. The complexity is
rather strange, though, as the bool would have been much easier to
understand.

I'll adapt the commit message.


Juergen