[PATCH] linux/x86: eliminate nesting of run-queue locks inside xtime_lock

[PATCH] linux/x86: eliminate nesting of run-queue locks inside xtime_lock

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

From: Zdenek Salvet <salvet@ics.muni.cz>

According to Debian bug 591362 this has been causing problems. While
no proof was given that the inverse lock order does actually occur
anywhere (with interrupts enabled), it is plain unnecessary to take
the risk.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Cc: Ian Campbell <ian.campbell@citrix.com>

--- a/arch/i386/kernel/time-xen.c
+++ b/arch/i386/kernel/time-xen.c
@@ -666,6 +666,7 @@ irqreturn_t timer_interrupt(int irq, voi
 	s64 delta, delta_cpu, stolen, blocked;
 	u64 sched_time;
 	unsigned int i, cpu = smp_processor_id();
+	int schedule_clock_was_set_work = 0;
 	struct shadow_time_info *shadow = &per_cpu(shadow_time, cpu);
 	struct vcpu_runstate_info *runstate = &per_cpu(runstate, cpu);
 
@@ -720,12 +721,14 @@ irqreturn_t timer_interrupt(int irq, voi
 
 	if (shadow_tv_version != HYPERVISOR_shared_info->wc_version) {
 		update_wallclock();
-		if (keventd_up())
-			schedule_work(&clock_was_set_work);
+		schedule_clock_was_set_work = 1;
 	}
 
 	write_sequnlock(&xtime_lock);
 
+	if (schedule_clock_was_set_work && keventd_up())
+		schedule_work(&clock_was_set_work);
+
 	/*
 	 * Account stolen ticks.
 	 * HACK: Passing NULL to account_steal_time()




[-- Attachment #2: xenlinux-x86-clock-was-set-work.patch --]
[-- Type: text/plain, Size: 1316 bytes --]

From: Zdenek Salvet <salvet@ics.muni.cz>
Subject: eliminate nesting of run-queue locks inside xtime_lock

According to Debian bug 591362 this has been causing problems. While
no proof was given that the inverse lock order does actually occur
anywhere (with interrupts enabled), it is plain unnecessary to take
the risk.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Cc: Ian Campbell <ian.campbell@citrix.com>

--- a/arch/i386/kernel/time-xen.c
+++ b/arch/i386/kernel/time-xen.c
@@ -666,6 +666,7 @@ irqreturn_t timer_interrupt(int irq, voi
 	s64 delta, delta_cpu, stolen, blocked;
 	u64 sched_time;
 	unsigned int i, cpu = smp_processor_id();
+	int schedule_clock_was_set_work = 0;
 	struct shadow_time_info *shadow = &per_cpu(shadow_time, cpu);
 	struct vcpu_runstate_info *runstate = &per_cpu(runstate, cpu);
 
@@ -720,12 +721,14 @@ irqreturn_t timer_interrupt(int irq, voi
 
 	if (shadow_tv_version != HYPERVISOR_shared_info->wc_version) {
 		update_wallclock();
-		if (keventd_up())
-			schedule_work(&clock_was_set_work);
+		schedule_clock_was_set_work = 1;
 	}
 
 	write_sequnlock(&xtime_lock);
 
+	if (schedule_clock_was_set_work && keventd_up())
+		schedule_work(&clock_was_set_work);
+
 	/*
 	 * Account stolen ticks.
 	 * HACK: Passing NULL to account_steal_time()

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] linux/x86: eliminate nesting of run-queue locks inside xtime_lock

[Ian Campbell]

On Tue, 2010-08-10 at 15:10 +0100, Jan Beulich wrote:
> From: Zdenek Salvet <salvet@ics.muni.cz>
> 
> According to Debian bug 591362 this has been causing problems. While
> no proof was given that the inverse lock order does actually occur
> anywhere (with interrupts enabled), it is plain unnecessary to take
> the risk.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

I'm right in thinking this is for traditional Xen/Linux kernel only,
aren't I?

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> --- a/arch/i386/kernel/time-xen.c
> +++ b/arch/i386/kernel/time-xen.c
> @@ -666,6 +666,7 @@ irqreturn_t timer_interrupt(int irq, voi
>  	s64 delta, delta_cpu, stolen, blocked;
>  	u64 sched_time;
>  	unsigned int i, cpu = smp_processor_id();
> +	int schedule_clock_was_set_work = 0;
>  	struct shadow_time_info *shadow = &per_cpu(shadow_time, cpu);
>  	struct vcpu_runstate_info *runstate = &per_cpu(runstate, cpu);
>  
> @@ -720,12 +721,14 @@ irqreturn_t timer_interrupt(int irq, voi
>  
>  	if (shadow_tv_version != HYPERVISOR_shared_info->wc_version) {
>  		update_wallclock();
> -		if (keventd_up())
> -			schedule_work(&clock_was_set_work);
> +		schedule_clock_was_set_work = 1;
>  	}
>  
>  	write_sequnlock(&xtime_lock);
>  
> +	if (schedule_clock_was_set_work && keventd_up())
> +		schedule_work(&clock_was_set_work);
> +
>  	/*
>  	 * Account stolen ticks.
>  	 * HACK: Passing NULL to account_steal_time()
> 
> 
> 

Re: [PATCH] linux/x86: eliminate nesting of run-queue locks inside xtime_lock

[Jan Beulich]

>>> On 10.08.10 at 16:25, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2010-08-10 at 15:10 +0100, Jan Beulich wrote:
>> From: Zdenek Salvet <salvet@ics.muni.cz>
>> 
>> According to Debian bug 591362 this has been causing problems. While
>> no proof was given that the inverse lock order does actually occur
>> anywhere (with interrupts enabled), it is plain unnecessary to take
>> the risk.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> I'm right in thinking this is for traditional Xen/Linux kernel only,
> aren't I?

Yes, absolutely.

Jan