Andrew Cooper
2013-Mar-21 23:19 UTC
[PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
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 */
Jan Beulich
2013-Mar-22 08:15 UTC
Re: [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
>>> 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
Keir Fraser
2013-Mar-22 08:20 UTC
Re: [PATCH] ACPI/APEI: Unlock apei_iomaps_lock on error path
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 >