| Message ID | 20171226125359.23706-2-joserz@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Increase dlpar initcall priority | expand |
Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> writes: > The hotplug engine uses its own workqueue to handle IRQ requests, the > problem is that such workqueue is initialized after init_ras_IRQ, which > will cause a kernel panic if any hotplug interruption is issued in that > period of time. > > This patch changes the dlpar initcall registration to make sure it will > be initialized before init_ras_IRQ. Sorry I know this is already v2, but I don't think this is the best fix. There's a dependency between the registration of the IRQ in the RAS code, and the creation of the work queue in the DLPAR code, but it's currently not explicit. That's the bug. So it'd be better to just make it explicit. As a bonus we can add actual error checking of the workqueue allocation. Something like below, can you test it please? cheers diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c index 6e35780c5962..dd8b29e58a98 100644 --- a/arch/powerpc/platforms/pseries/dlpar.c +++ b/arch/powerpc/platforms/pseries/dlpar.c @@ -574,11 +574,26 @@ static ssize_t dlpar_show(struct class *class, struct class_attribute *attr, static CLASS_ATTR_RW(dlpar); -static int __init pseries_dlpar_init(void) +int __init dlpar_workqueue_init(void) { + if (pseries_hp_wq) + return 0; + pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue", WQ_UNBOUND, 1); + + return pseries_hp_wq ? 0 : -ENOMEM; +} + +static int __init dlpar_sysfs_init(void) +{ + int rc; + + rc = dlpar_workqueue_init(); + if (rc) + return rc; + return sysfs_create_file(kernel_kobj, &class_attr_dlpar.attr); } -machine_device_initcall(pseries, pseries_dlpar_init); +machine_device_initcall(pseries, dlpar_sysfs_init); diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 4470a3194311..1ae1d9f4dbe9 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -98,4 +98,6 @@ static inline unsigned long cmo_get_page_size(void) return CMO_PageSize; } +int dlpar_workqueue_init(void); + #endif /* _PSERIES_PSERIES_H */ diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c index 4923ffe230cf..879a92327010 100644 --- a/arch/powerpc/platforms/pseries/ras.c +++ b/arch/powerpc/platforms/pseries/ras.c @@ -69,8 +69,9 @@ static int __init init_ras_IRQ(void) /* Hotplug Events */ np = of_find_node_by_path("/event-sources/hot-plug-events"); if (np != NULL) { - request_event_sources_irqs(np, ras_hotplug_interrupt, - "RAS_HOTPLUG"); + if (dlpar_workqueue_init() == 0) + request_event_sources_irqs(np, ras_hotplug_interrupt, + "RAS_HOTPLUG"); of_node_put(np); }
On Tue, Jan 02, 2018 at 10:46:07PM +1100, Michael Ellerman wrote: > Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> writes: > > > The hotplug engine uses its own workqueue to handle IRQ requests, the > > problem is that such workqueue is initialized after init_ras_IRQ, which > > will cause a kernel panic if any hotplug interruption is issued in that > > period of time. > > > > This patch changes the dlpar initcall registration to make sure it will > > be initialized before init_ras_IRQ. > > Sorry I know this is already v2, but I don't think this is the best fix. > > There's a dependency between the registration of the IRQ in the RAS > code, and the creation of the work queue in the DLPAR code, but it's > currently not explicit. That's the bug. So it'd be better to just make > it explicit. > > As a bonus we can add actual error checking of the workqueue allocation. > > Something like below, can you test it please? sure, no problem. I'll do it. Thanks for reviewing it!!! > > cheers > > diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c > index 6e35780c5962..dd8b29e58a98 100644 > --- a/arch/powerpc/platforms/pseries/dlpar.c > +++ b/arch/powerpc/platforms/pseries/dlpar.c > @@ -574,11 +574,26 @@ static ssize_t dlpar_show(struct class *class, struct class_attribute *attr, > > static CLASS_ATTR_RW(dlpar); > > -static int __init pseries_dlpar_init(void) > +int __init dlpar_workqueue_init(void) > { > + if (pseries_hp_wq) > + return 0; > + > pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue", > WQ_UNBOUND, 1); > + > + return pseries_hp_wq ? 0 : -ENOMEM; > +} > + > +static int __init dlpar_sysfs_init(void) > +{ > + int rc; > + > + rc = dlpar_workqueue_init(); > + if (rc) > + return rc; > + > return sysfs_create_file(kernel_kobj, &class_attr_dlpar.attr); > } > -machine_device_initcall(pseries, pseries_dlpar_init); > +machine_device_initcall(pseries, dlpar_sysfs_init); > > diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h > index 4470a3194311..1ae1d9f4dbe9 100644 > --- a/arch/powerpc/platforms/pseries/pseries.h > +++ b/arch/powerpc/platforms/pseries/pseries.h > @@ -98,4 +98,6 @@ static inline unsigned long cmo_get_page_size(void) > return CMO_PageSize; > } > > +int dlpar_workqueue_init(void); > + > #endif /* _PSERIES_PSERIES_H */ > diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c > index 4923ffe230cf..879a92327010 100644 > --- a/arch/powerpc/platforms/pseries/ras.c > +++ b/arch/powerpc/platforms/pseries/ras.c > @@ -69,8 +69,9 @@ static int __init init_ras_IRQ(void) > /* Hotplug Events */ > np = of_find_node_by_path("/event-sources/hot-plug-events"); > if (np != NULL) { > - request_event_sources_irqs(np, ras_hotplug_interrupt, > - "RAS_HOTPLUG"); > + if (dlpar_workqueue_init() == 0) > + request_event_sources_irqs(np, ras_hotplug_interrupt, > + "RAS_HOTPLUG"); > of_node_put(np); > } > >
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c index 6e35780c5962..1f9ae1d0b2be 100644 --- a/arch/powerpc/platforms/pseries/dlpar.c +++ b/arch/powerpc/platforms/pseries/dlpar.c @@ -580,5 +580,5 @@ static int __init pseries_dlpar_init(void) WQ_UNBOUND, 1); return sysfs_create_file(kernel_kobj, &class_attr_dlpar.attr); } -machine_device_initcall(pseries, pseries_dlpar_init); +machine_arch_initcall(pseries, pseries_dlpar_init);
The hotplug engine uses its own workqueue to handle IRQ requests, the problem is that such workqueue is initialized after init_ras_IRQ, which will cause a kernel panic if any hotplug interruption is issued in that period of time. This patch changes the dlpar initcall registration to make sure it will be initialized before init_ras_IRQ. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- arch/powerpc/platforms/pseries/dlpar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)