[PATCH 1/3] x86: apic, restore irqs on fail paths

[PATCH 1/3] x86: apic, restore irqs on fail paths

[Jiri Slaby]

lapic_resume forgets to restore interrupts on fail paths.
Fix that.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/apic/apic.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 07cffc1..fc65544 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2029,7 +2029,7 @@ static int lapic_resume(struct sys_device *dev)
 	unsigned int l, h;
 	unsigned long flags;
 	int maxlvt;
-	int ret;
+	int ret = 0;
 	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
 	if (!apic_pm_state.active)
@@ -2040,14 +2040,15 @@ static int lapic_resume(struct sys_device *dev)
 		ioapic_entries = alloc_ioapic_entries();
 		if (!ioapic_entries) {
 			WARN(1, "Alloc ioapic_entries in lapic resume failed.");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto restore;
 		}
 
 		ret = save_IO_APIC_setup(ioapic_entries);
 		if (ret) {
 			WARN(1, "Saving IO-APIC state failed: %d\n", ret);
 			free_ioapic_entries(ioapic_entries);
-			return ret;
+			goto restore;
 		}
 
 		mask_IO_APIC_setup(ioapic_entries);
@@ -2099,10 +2100,10 @@ static int lapic_resume(struct sys_device *dev)
 		restore_IO_APIC_setup(ioapic_entries);
 		free_ioapic_entries(ioapic_entries);
 	}
-
+restore:
 	local_irq_restore(flags);
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
1.6.3

[PATCH 2/3] x86: amd_iommu, fix lock imbalance

[Jiri Slaby]

In alloc_coherent there is an omitted unlock on the path where mapping
fails. Add the unlock.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index a97db99..b123e20 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1537,8 +1537,10 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	*dma_addr = __map_single(dev, iommu, domain->priv, paddr,
 				 size, DMA_BIDIRECTIONAL, true, dma_mask);
 
-	if (*dma_addr == bad_dma_address)
+	if (*dma_addr == bad_dma_address) {
+		spin_unlock_irqrestore(&domain->lock, flags);
 		goto out_free;
+	}
 
 	iommu_completion_wait(iommu);
 
-- 
1.6.3

[PATCH 3/3] trace: events_filter, fix lock imbalances

[Jiri Slaby]

apply_event_filter and apply_subsystem_event_filter omits an unlock
on one fail path each. Fix that.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
---
 kernel/trace/trace_events_filter.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 85ad6a8..e35094a 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1080,8 +1080,10 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string)
 	}
 
 	ps = kzalloc(sizeof(*ps), GFP_KERNEL);
-	if (!ps)
-		return -ENOMEM;
+	if (!ps) {
+		err = -ENOMEM;
+		goto unlock;
+	}
 
 	filter_disable_preds(call);
 	replace_filter_string(call->filter, filter_string);
@@ -1101,7 +1103,7 @@ out:
 	filter_opstack_clear(ps);
 	postfix_clear(ps);
 	kfree(ps);
-
+unlock:
 	mutex_unlock(&filter_mutex);
 
 	return err;
@@ -1124,8 +1126,10 @@ int apply_subsystem_event_filter(struct event_subsystem *system,
 	}
 
 	ps = kzalloc(sizeof(*ps), GFP_KERNEL);
-	if (!ps)
-		return -ENOMEM;
+	if (!ps) {
+		err = -ENOMEM;
+		goto unlock;
+	}
 
 	filter_free_subsystem_preds(system);
 	replace_filter_string(system->filter, filter_string);
@@ -1145,7 +1149,7 @@ out:
 	filter_opstack_clear(ps);
 	postfix_clear(ps);
 	kfree(ps);
-
+unlock:
 	mutex_unlock(&filter_mutex);
 
 	return err;
-- 
1.6.3

Re: [PATCH 3/3] trace: events_filter, fix lock imbalances

[Li Zefan]

Jiri Slaby wrote:
> apply_event_filter and apply_subsystem_event_filter omits an unlock
> on one fail path each. Fix that.
> 

Thanks, but this has been fixed.

You are not using latest tip tree, are you?

> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: Tom Zanussi <tzanussi@gmail.com>

Re: [PATCH 3/3] trace: events_filter, fix lock imbalances

[Jiri Slaby]

On 05/29/2009 03:14 AM, Li Zefan wrote:
> Jiri Slaby wrote:
>> apply_event_filter and apply_subsystem_event_filter omits an unlock
>> on one fail path each. Fix that.
> 
> Thanks, but this has been fixed.

Ok, sorry, I didn't notice.

Re: [PATCH 1/3] x86: apic, restore irqs on fail paths

[Cyrill Gorcunov]

[Jiri Slaby - Thu, May 28, 2009 at 09:54:47AM +0200]
|
| lapic_resume forgets to restore interrupts on fail paths.
| Fix that.

Good catch, Jiri!

Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>

Though I really doubt if the system will be functional after such a fail :)
But it's definitely better then having it irq-locked.

Yinghai CC'ed just to be sure.

| 
| Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
| Cc: Ingo Molnar <mingo@redhat.com>
| Cc: Thomas Gleixner <tglx@linutronix.de>
| Cc: H. Peter Anvin <hpa@zytor.com>
| ---
|  arch/x86/kernel/apic/apic.c |   11 ++++++-----
|  1 files changed, 6 insertions(+), 5 deletions(-)
| 
| diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
| index 07cffc1..fc65544 100644
| --- a/arch/x86/kernel/apic/apic.c
| +++ b/arch/x86/kernel/apic/apic.c
| @@ -2029,7 +2029,7 @@ static int lapic_resume(struct sys_device *dev)
|  	unsigned int l, h;
|  	unsigned long flags;
|  	int maxlvt;
| -	int ret;
| +	int ret = 0;
|  	struct IO_APIC_route_entry **ioapic_entries = NULL;
|  
|  	if (!apic_pm_state.active)
| @@ -2040,14 +2040,15 @@ static int lapic_resume(struct sys_device *dev)
|  		ioapic_entries = alloc_ioapic_entries();
|  		if (!ioapic_entries) {
|  			WARN(1, "Alloc ioapic_entries in lapic resume failed.");
| -			return -ENOMEM;
| +			ret = -ENOMEM;
| +			goto restore;
|  		}
|  
|  		ret = save_IO_APIC_setup(ioapic_entries);
|  		if (ret) {
|  			WARN(1, "Saving IO-APIC state failed: %d\n", ret);
|  			free_ioapic_entries(ioapic_entries);
| -			return ret;
| +			goto restore;
|  		}
|  
|  		mask_IO_APIC_setup(ioapic_entries);
| @@ -2099,10 +2100,10 @@ static int lapic_resume(struct sys_device *dev)
|  		restore_IO_APIC_setup(ioapic_entries);
|  		free_ioapic_entries(ioapic_entries);
|  	}
| -
| +restore:
|  	local_irq_restore(flags);
|  
| -	return 0;
| +	return ret;
|  }
|  
|  /*
| -- 
| 1.6.3
| 
| 
	-- Cyrill

[tip:irq/numa] x86, apic: Restore irqs on fail paths

[tip-bot for Jiri Slaby]

Commit-ID:  3d58829b0510244596079c1d2f1762c53aef2e97
Gitweb:     http://git.kernel.org/tip/3d58829b0510244596079c1d2f1762c53aef2e97
Author:     Jiri Slaby <jirislaby@gmail.com>
AuthorDate: Thu, 28 May 2009 09:54:47 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 2 Jun 2009 02:48:59 +0200

x86, apic: Restore irqs on fail paths

lapic_resume forgets to restore interrupts on fail paths.
Fix that.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
LKML-Reference: <1243497289-18591-1-git-send-email-jirislaby@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>


---
 arch/x86/kernel/apic/apic.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b0fd264..e82488d 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2027,7 +2027,7 @@ static int lapic_resume(struct sys_device *dev)
 	unsigned int l, h;
 	unsigned long flags;
 	int maxlvt;
-	int ret;
+	int ret = 0;
 	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
 	if (!apic_pm_state.active)
@@ -2038,14 +2038,15 @@ static int lapic_resume(struct sys_device *dev)
 		ioapic_entries = alloc_ioapic_entries();
 		if (!ioapic_entries) {
 			WARN(1, "Alloc ioapic_entries in lapic resume failed.");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto restore;
 		}
 
 		ret = save_IO_APIC_setup(ioapic_entries);
 		if (ret) {
 			WARN(1, "Saving IO-APIC state failed: %d\n", ret);
 			free_ioapic_entries(ioapic_entries);
-			return ret;
+			goto restore;
 		}
 
 		mask_IO_APIC_setup(ioapic_entries);
@@ -2097,10 +2098,10 @@ static int lapic_resume(struct sys_device *dev)
 		restore_IO_APIC_setup(ioapic_entries);
 		free_ioapic_entries(ioapic_entries);
 	}
-
+restore:
 	local_irq_restore(flags);
 
-	return 0;
+	return ret;
 }
 
 /*

Re: [PATCH 2/3] x86: amd_iommu, fix lock imbalance

[Joerg Roedel]

On Thu, May 28, 2009 at 09:54:48AM +0200, Jiri Slaby wrote:
> In alloc_coherent there is an omitted unlock on the path where mapping
> fails. Add the unlock.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kernel/amd_iommu.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> index a97db99..b123e20 100644
> --- a/arch/x86/kernel/amd_iommu.c
> +++ b/arch/x86/kernel/amd_iommu.c
> @@ -1537,8 +1537,10 @@ static void *alloc_coherent(struct device *dev, size_t size,
>  	*dma_addr = __map_single(dev, iommu, domain->priv, paddr,
>  				 size, DMA_BIDIRECTIONAL, true, dma_mask);
>  
> -	if (*dma_addr == bad_dma_address)
> +	if (*dma_addr == bad_dma_address) {
> +		spin_unlock_irqrestore(&domain->lock, flags);
>  		goto out_free;
> +	}
>  
>  	iommu_completion_wait(iommu);

Applied to amd-iommu/fixes. Thanks Jiri.

-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632