LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Alex Elder <elder@linaro.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Andy Gross <agross@kernel.org>, Vinod Koul <vkoul@kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] soc: qcom: aoss: qmp_send() can't handle interruptions so stop pretending
Date: Wed, 5 Aug 2020 13:25:27 -0700
Message-ID: <CAD=FV=V9K17Un9bPnUvieNO_6tpThPVqdk92_KcQxpOtMTYDxg@mail.gmail.com> (raw)
In-Reply-To: <159664848938.1360974.193633020977562116@swboyd.mtv.corp.google.com>

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

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 16:16 [PATCH 1/2] soc: qcom: aoss: Don't wait for IRQ if we might be in suspend/resume noirq Douglas Anderson
2020-08-05 16:16 ` [PATCH 2/2] soc: qcom: aoss: qmp_send() can't handle interruptions so stop pretending Douglas Anderson
2020-08-05 17:28   ` Stephen Boyd
2020-08-05 20:25     ` Doug Anderson [this message]
2020-08-05 17:36 ` [PATCH 1/2] soc: qcom: aoss: Don't wait for IRQ if we might be in suspend/resume noirq Stephen Boyd
2020-08-05 20:24   ` Doug Anderson
2020-08-05 23:02     ` Stephen Boyd
2020-08-06 14:34       ` Sibi Sankar
2020-08-06 17:10         ` Doug Anderson
2020-08-06 17:33           ` Sibi Sankar
2020-08-11 21:21             ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD=FV=V9K17Un9bPnUvieNO_6tpThPVqdk92_KcQxpOtMTYDxg@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=elder@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=swboyd@chromium.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git