On Fri, Dec 22, 2017 at 11:54:10AM +1100, David Gibson wrote:
> 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 <[email protected]>
>
> 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.
>
Oh, this makes much more sense. I'm going to make some tests and send a
v2 then.
Thanks for reviewing it!
> > ---
> > 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
