Re: [PATCH 2/2] soc: qcom: aoss: qmp_send() can't handle interruptions so stop pretending

[Doug Anderson <dianders@chromium.org>]

Hi,

On Wed, Aug 5, 2020 at 10:28 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Douglas Anderson (2020-08-05 09:16:11)
> > The function qmp_send() called wait_event_interruptible_timeout() to
> > wait for its interrupt.  However, this function did not check for
> > -ERESTARTSYS and assumed that any non-zero return value meant that the
> > event happened.
> >
> > While we could try to figure out how to handle interruptions by
> > figuring out how to cancel and/or undo our transfer in a race-free way
> > and then communicating this status back to all of our callers, that
> > seems like a whole lot of complexity.  As I understand it the transfer
> > should happen rather quickly and if we're really hitting the 1 second
> > timeout we're in deep doggy doodoo anyway.  Let's just use the
> > non-interruptible version of the function and call it good enough.
> >
> > Found by code inspection.  No known test cases expose the problem
> > described here.
> >
> > Fixes: 2209481409b7 ("soc: qcom: Add AOSS QMP driver")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
>
> I would put this first in the series as it's an obvious bug fix.

I guess I thought of it the other way.  This is a less serious problem
(no known way to tickle it) and so deserved to be 2nd.  I'm happy to
flip it as needed, though.  It would also be trivially easy to flip
when applying.


> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Thanks!

-Doug