| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140131160140.GC23648@phenom.dumpdata.com>
Date: Fri, 31 Jan 2014 11:01:40 -0500
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Stanislaw Gruszka <sgruszka@...hat.com>
Cc: David Rientjes <rientjes@...gle.com>, boris.ostrovsky@...cle.com,
david.vrabel@...rix.com, Ben Guthro <benjamin.guthro@...rix.com>,
linux-kernel@...r.kernel.org,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
xen-devel@...ts.xensource.com
Subject: Re: [BUG?] Interrupts enabled after
xen_acpi_processor_resume+0x0/0x34 [xen_acpi_processor]
On Wed, Jan 29, 2014 at 09:25:22AM +0100, Stanislaw Gruszka wrote:
> (Cc: correct Rafael email)
>
> On Tue, Jan 28, 2014 at 09:24:57PM -0800, David Rientjes wrote:
> > On Tue, 28 Jan 2014, Konrad Rzeszutek Wilk wrote:
> >
> > > diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
> > > index 7231859..7602229 100644
> > > --- a/drivers/xen/xen-acpi-processor.c
> > > +++ b/drivers/xen/xen-acpi-processor.c
> > > @@ -46,7 +46,7 @@ module_param_named(off, no_hypercall, int, 0400);
> > > */
> > > static unsigned int nr_acpi_bits;
> > > /* Mutex to protect the acpi_ids_done - for CPU hotplug use. */
> > > -static DEFINE_MUTEX(acpi_ids_mutex);
> > > +static DEFINE_SPINLOCK(acpi_ids_lock);
> > > /* Which ACPI ID we have processed from 'struct acpi_processor'. */
> > > static unsigned long *acpi_ids_done;
> > > /* Which ACPI ID exist in the SSDT/DSDT processor definitions. */
> > > @@ -68,7 +68,7 @@ static int push_cxx_to_hypervisor(struct acpi_processor *_pr)
> > > int ret = 0;
> > >
> > > dst_cx_states = kcalloc(_pr->power.count,
> > > - sizeof(struct xen_processor_cx), GFP_KERNEL);
> > > + sizeof(struct xen_processor_cx), GFP_ATOMIC);
> > > if (!dst_cx_states)
> > > return -ENOMEM;
> > >
> > > @@ -149,7 +149,7 @@ xen_copy_pss_data(struct acpi_processor *_pr,
> > > sizeof(struct acpi_processor_px));
> > >
> > > dst_states = kcalloc(_pr->performance->state_count,
> > > - sizeof(struct xen_processor_px), GFP_KERNEL);
> > > + sizeof(struct xen_processor_px), GFP_ATOMIC);
> > > if (!dst_states)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > @@ -275,9 +275,9 @@ static int upload_pm_data(struct acpi_processor *_pr)
> > > {
> > > int err = 0;
> > >
> > > - mutex_lock(&acpi_ids_mutex);
> > > + spin_lock(&acpi_ids_lock);
> > > if (__test_and_set_bit(_pr->acpi_id, acpi_ids_done)) {
> > > - mutex_unlock(&acpi_ids_mutex);
> > > + spin_unlock(&acpi_ids_lock);
> > > return -EBUSY;
> > > }
> > > if (_pr->flags.power)
> > > @@ -286,7 +286,7 @@ static int upload_pm_data(struct acpi_processor *_pr)
> > > if (_pr->performance && _pr->performance->states)
> > > err |= push_pxx_to_hypervisor(_pr);
> > >
> > > - mutex_unlock(&acpi_ids_mutex);
> > > + spin_unlock(&acpi_ids_lock);
> > > return err;
> > > }
> > > static unsigned int __init get_max_acpi_id(void)
> >
> > Looks incomplete, what about the kzalloc() in
> > xen_upload_processor_pm_data() and kcalloc()s in check_acpi_ids()?
>
> Indeed and additionally from check_acpi_ids() we call
> acpi_walk_namespace(), which also take mutexes. Hence unfortunately
> making xen_upload_processor_pm_data() atomic is not easy, but possibly
> can be done by saving some data in memory after initialization.
>
> Or perhaps this problem can be solved differently, by not using
> yscore_ops->resume(), but some other resume callback from core, which
> allow to sleep. Than can require registering dummy device or sysfs
> class, but maybe there are simpler solutions.
Perhaps by using 'subsys_system_register' and stick it there?
Again, untested and uncompiled:
diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
index 7231859..a2524f6 100644
--- a/drivers/xen/xen-acpi-processor.c
+++ b/drivers/xen/xen-acpi-processor.c
@@ -27,7 +27,6 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/types.h>
-#include <linux/syscore_ops.h>
#include <linux/acpi.h>
#include <acpi/processor.h>
#include <xen/xen.h>
@@ -495,13 +494,15 @@ static int xen_upload_processor_pm_data(void)
return rc;
}
-static void xen_acpi_processor_resume(void)
+static int xen_acpi_processor_resume(struct device *dev)
{
bitmap_zero(acpi_ids_done, nr_acpi_bits);
- xen_upload_processor_pm_data();
+ return xen_upload_processor_pm_data();
}
-static struct syscore_ops xap_syscore_ops = {
+static struct bus_type xap_bus = {
+ .name = "xen-acpi-processor",
+ .dev_name = "xen-acpi-processor",
.resume = xen_acpi_processor_resume,
};
@@ -555,7 +556,7 @@ static int __init xen_acpi_processor_init(void)
if (rc)
goto err_unregister;
- register_syscore_ops(&xap_syscore_ops);
+ subsys_system_register(&xap_bus);
return 0;
err_unregister:
@@ -574,7 +575,7 @@ static void __exit xen_acpi_processor_exit(void)
{
int i;
- unregister_syscore_ops(&xap_syscore_ops);
+ bus_unregister(&xap_bus);
kfree(acpi_ids_done);
kfree(acpi_id_present);
kfree(acpi_id_cst_present);
>
> Thanks
> Stanislaw
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists