On Tue, Jan 02, 2018 at 10:46:07PM +1100, Michael Ellerman wrote:
> Jose Ricardo Ziviani <jos...@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);
> }
>
>
