From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Alex Elder <elder@linaro.org>
Cc: ohad@wizery.com, mathieu.poirier@linaro.org,
linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] remoteproc: use freezable workqueue for crash notifications
Date: Thu, 27 May 2021 22:55:05 -0500 [thread overview]
Message-ID: <YLBpmdZoGDXNz64y@builder.lan> (raw)
In-Reply-To: <20210519234418.1196387-2-elder@linaro.org>
On Wed 19 May 18:44 CDT 2021, Alex Elder wrote:
> When a remoteproc has crashed, rproc_report_crash() is called to
> handle whatever recovery is desired. This can happen at almost any
> time, often triggered by an interrupt, though it can also be
> initiated by a write to debugfs file remoteproc/remoteproc*/crash.
>
> When a crash is reported, the crash handler worker is scheduled to
> run (rproc_crash_handler_work()). One thing that worker does is
> call rproc_trigger_recovery(), which calls rproc_stop(). That calls
> the ->stop method for any remoteproc subdevices before making the
> remote processor go offline.
>
> The Q6V5 modem remoteproc driver implements an SSR subdevice that
> notifies registered drivers when the modem changes operational state
> (prepare, started, stop/crash, unprepared). The IPA driver
> registers to receive these notifications.
>
> With that as context, I'll now describe the problem.
>
> There was a situation in which buggy modem firmware led to a modem
> crash very soon after system (AP) resume had begun. The crash caused
> a remoteproc SSR crash notification to be sent to the IPA driver.
> The problem was that, although system resume had begun, it had not
> yet completed, and the IPA driver was still in a suspended state.
>
> This scenario could happen to any driver that registers for these
> SSR notifications, because they are delivered without knowledge of
> the (suspend) state of registered recipient drivers.
>
> This patch offers a simple fix for this, by having the crash
> handling worker function run on the system freezable workqueue.
> This workqueue does not operate if user space is frozen (for
> suspend). As a result, the SSR subdevice only delivers its
> crash notification when the system is fully operational (i.e.,
> neither suspended nor in suspend/resume transition).
>
This makes sense to me; both that it ensures that we spend our resources
on the actual system resume and that it avoids surprises from this
happening while the system still is in a funky state...
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
But it would be nice to get some input from other users of the
framework.
Regards,
Bjorn
> Signed-off-by: Alex Elder <elder@linaro.org>
> ---
> drivers/remoteproc/remoteproc_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 39cf44cb08035..6bedf2d2af239 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2724,8 +2724,8 @@ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type)
> dev_err(&rproc->dev, "crash detected in %s: type %s\n",
> rproc->name, rproc_crash_to_string(type));
>
> - /* create a new task to handle the error */
> - schedule_work(&rproc->crash_handler);
> + /* Have a worker handle the error; ensure system is not suspended */
> + queue_work(system_freezable_wq, &rproc->crash_handler);
> }
> EXPORT_SYMBOL(rproc_report_crash);
>
> --
> 2.27.0
>
next prev parent reply other threads:[~2021-05-28 3:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-19 23:44 [PATCH 0/1] remoteproc: avoid notification when suspended Alex Elder
2021-05-19 23:44 ` [PATCH 1/1] remoteproc: use freezable workqueue for crash notifications Alex Elder
2021-05-28 3:55 ` Bjorn Andersson [this message]
2021-05-28 15:09 ` Mathieu Poirier
2021-05-29 0:12 ` Siddharth Gupta
2021-06-04 20:46 ` Siddharth Gupta
[not found] ` <20210529024847.5164-1-hdanton@sina.com>
2021-05-29 17:28 ` Bjorn Andersson
[not found] ` <20210530030728.8340-1-hdanton@sina.com>
2021-05-31 23:25 ` Bjorn Andersson
2021-05-31 17:21 ` Mathieu Poirier
2021-05-31 23:13 ` Bjorn Andersson
2021-06-01 14:12 ` Alex Elder
2021-08-04 19:31 ` [PATCH 0/1] remoteproc: avoid notification when suspended patchwork-bot+linux-remoteproc
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=YLBpmdZoGDXNz64y@builder.lan \
--to=bjorn.andersson@linaro.org \
--cc=elder@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=ohad@wizery.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).