On Thu, Mar 19, 2020 at 10:00:24AM +0100, Sebastian Andrzej Siewior wrote:
> On 2020-03-18 21:43:09 [+0100], Thomas Gleixner wrote:
> > --- a/arch/powerpc/platforms/ps3/device-init.c
> > +++ b/arch/powerpc/platforms/ps3/device-init.c
> > @@ -725,12 +728,12 @@ static int ps3_notification_read_write(s
> > unsigned long flags;
> > int res;
> >
> > - init_completion(&dev->done);
> > spin_lock_irqsave(&dev->lock, flags);
> > res = write ? lv1_storage_write(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> > &dev->tag)
> > : lv1_storage_read(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> > &dev->tag);
> > + dev->done = false;
> > spin_unlock_irqrestore(&dev->lock, flags);
> > if (res) {
> > pr_err("%s:%u: %s failed %d\n", __func__, __LINE__, op, res);
> > @@ -738,14 +741,10 @@ static int ps3_notification_read_write(s
> > }
> > pr_debug("%s:%u: notification %s issued\n", __func__, __LINE__, op);
> >
> > - res = wait_event_interruptible(dev->done.wait,
> > - dev->done.done || kthread_should_stop());
> > + rcuwait_wait_event(&dev->wait, dev->done || kthread_should_stop(),
> > TASK_IDLE);
> > +
> …
>
> Not sure it matters but this struct `dev' is allocated on stack. Should
> the interrupt fire *before* rcuwait_wait_event() set wait.task to NULL
> then it is of random value on the first invocation of rcuwait_wake_up().
> ->
>
> diff --git a/arch/powerpc/platforms/ps3/device-init.c
> b/arch/powerpc/platforms/ps3/device-init.c
> index 197347c3c0b24..e87360a0fb40d 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -809,6 +809,7 @@ static int ps3_probe_thread(void *data)
> }
>
> spin_lock_init(&dev.lock);
> + rcuwait_init(&dev.wait);
>
> res = request_irq(irq, ps3_notification_interrupt, 0,
> "ps3_notification", &dev);
>
Very good, sorry for that.
