Re: [PATCH 1/1] powerpc/pseries: Use the system workqueue as fallback to hotplug workqueue

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]

On Thu, Dec 21, 2017 at 01:44:48PM -0200, Jose Ricardo Ziviani wrote:
> The hotplug engine uses its own workqueue to handle IRQ requests, the
> problem is that such workqueue is initialized not so early in the boot
> process.
> 
> Thus, when the kernel is ready to handle IRQ requests, after the system
> workqueue is initialized, we have a timeframe where any hotplug issued
> by the client will result in a kernel panic. That timeframe goes until
> the hotplug workqueue is initialized.
> 
> It would be good to have the hotplug workqueue initialized as soon as
> the system workqueue but I don't think it is possible. So, this patch
> uses the system workqueue as a fallback the handle such IRQs.
> 
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>

I don't think this is the right approach.

It seems to me the bug is that the hotplug interrupt is registered in
init_ras_IRQ(), before the work queue is initialized in
pseries_dlpar_init().  We need to correct that ordering.

> ---
>  arch/powerpc/platforms/pseries/dlpar.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index 6e35780c5962..0474aa14b5f6 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -399,7 +399,15 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>  		work->errlog = hp_errlog_copy;
>  		work->hp_completion = hotplug_done;
>  		work->rc = rc;
> -		queue_work(pseries_hp_wq, (struct work_struct *)work);
> +
> +		/* The hotplug workqueue may happen to be NULL at the moment
> +		 * this code is executed, during the boot phase. So, in this
> +		 * scenario, we can fallback to the system workqueue.
> +		 */
> +		if (unlikely(pseries_hp_wq == NULL))
> +			schedule_work((struct work_struct *)work);
> +		else
> +			queue_work(pseries_hp_wq, (struct work_struct *)work);
>  	} else {
>  		*rc = -ENOMEM;
>  		kfree(hp_errlog_copy);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]