xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
@ 2013-03-21 23:19 Andrew Cooper
  2013-03-22  8:15 ` Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2013-03-21 23:19 UTC (permalink / raw)
  To: xen-devel; +Cc: Keir Fraser, Jan Beulich

This causes deadlocks during early boot on hardware with broken/buggy APEI
implementations, such as a Dell Poweredge 2950 with the latest currently
available BIOS.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 7babbd46586b -r 6a952c14c4dc xen/drivers/acpi/apei/apei-io.c
--- a/xen/drivers/acpi/apei/apei-io.c
+++ b/xen/drivers/acpi/apei/apei-io.c
@@ -147,12 +147,15 @@ static void __init apei_post_unmap(paddr
 	spin_lock_irqsave(&apei_iomaps_lock, flags);
 	map = __apei_find_iomap(paddr, size);
 	if (!map)
-		return;
+		goto err;
 
 	list_del(&map->list);
 	spin_unlock_irqrestore(&apei_iomaps_lock, flags);
 
 	xfree(map);
+	return;
+err:
+	spin_unlock_irqrestore(&apei_iomaps_lock, flags);
 }
 
 /* In NMI handler, should set silent = 1 */

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
  2013-03-21 23:19 [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path Andrew Cooper
@ 2013-03-22  8:15 ` Jan Beulich
  2013-03-22  8:20   ` Keir Fraser
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2013-03-22  8:15 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Keir Fraser, xen-devel

>>> On 22.03.13 at 00:19, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> This causes deadlocks during early boot on hardware with broken/buggy APEI
> implementations, such as a Dell Poweredge 2950 with the latest currently
> available BIOS.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> diff -r 7babbd46586b -r 6a952c14c4dc xen/drivers/acpi/apei/apei-io.c
> --- a/xen/drivers/acpi/apei/apei-io.c
> +++ b/xen/drivers/acpi/apei/apei-io.c
> @@ -147,12 +147,15 @@ static void __init apei_post_unmap(paddr
>  	spin_lock_irqsave(&apei_iomaps_lock, flags);
>  	map = __apei_find_iomap(paddr, size);
>  	if (!map)
> -		return;
> +		goto err;
>  
>  	list_del(&map->list);
>  	spin_unlock_irqrestore(&apei_iomaps_lock, flags);
>  
>  	xfree(map);
> +	return;
> +err:
> +	spin_unlock_irqrestore(&apei_iomaps_lock, flags);
>  }
>  
>  /* In NMI handler, should set silent = 1 */

I'd prefer doing this slightly differently:

--- a/xen/drivers/acpi/apei/apei-io.c
+++ b/xen/drivers/acpi/apei/apei-io.c
@@ -146,10 +146,8 @@ static void __init apei_post_unmap(paddr
 
 	spin_lock_irqsave(&apei_iomaps_lock, flags);
 	map = __apei_find_iomap(paddr, size);
-	if (!map)
-		return;
-
-	list_del(&map->list);
+	if (map)
+		list_del(&map->list);
 	spin_unlock_irqrestore(&apei_iomaps_lock, flags);
 
 	xfree(map);

Jan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
  2013-03-22  8:15 ` Jan Beulich
@ 2013-03-22  8:20   ` Keir Fraser
  0 siblings, 0 replies; 3+ messages in thread
From: Keir Fraser @ 2013-03-22  8:20 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper; +Cc: xen-devel

On 22/03/2013 08:15, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 22.03.13 at 00:19, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> This causes deadlocks during early boot on hardware with broken/buggy APEI
>> implementations, such as a Dell Poweredge 2950 with the latest currently
>> available BIOS.
>> 
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> diff -r 7babbd46586b -r 6a952c14c4dc xen/drivers/acpi/apei/apei-io.c
>> --- a/xen/drivers/acpi/apei/apei-io.c
>> +++ b/xen/drivers/acpi/apei/apei-io.c
>> @@ -147,12 +147,15 @@ static void __init apei_post_unmap(paddr
>> spin_lock_irqsave(&apei_iomaps_lock, flags);
>> map = __apei_find_iomap(paddr, size);
>> if (!map)
>> -  return;
>> +  goto err;
>>  
>> list_del(&map->list);
>> spin_unlock_irqrestore(&apei_iomaps_lock, flags);
>>  
>> xfree(map);
>> + return;
>> +err:
>> + spin_unlock_irqrestore(&apei_iomaps_lock, flags);
>>  }
>>  
>>  /* In NMI handler, should set silent = 1 */
> 
> I'd prefer doing this slightly differently:

In this case, agreed.

 -- Keir

> --- a/xen/drivers/acpi/apei/apei-io.c
> +++ b/xen/drivers/acpi/apei/apei-io.c
> @@ -146,10 +146,8 @@ static void __init apei_post_unmap(paddr
>  
> spin_lock_irqsave(&apei_iomaps_lock, flags);
> map = __apei_find_iomap(paddr, size);
> - if (!map)
> -  return;
> -
> - list_del(&map->list);
> + if (map)
> +  list_del(&map->list);
> spin_unlock_irqrestore(&apei_iomaps_lock, flags);
>  
> xfree(map);
> 
> Jan
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-03-22  8:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-21 23:19 [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path Andrew Cooper
2013-03-22  8:15 ` Jan Beulich
2013-03-22  8:20   ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).