2022-07-05 12:20:42

by Sibi Sankar

[permalink] [raw]
Subject: [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs

From: Siddharth Gupta <[email protected]>

When a new remoteproc boots up, send the sysmon state notification
of only running remoteprocs. Sending state of remoteprocs booting
up in parallel can cause a race between SSR clients of the remoteproc
that is booting up and the sysmon notification for the same remoteproc,
resulting in an inconsistency between which state the remoteproc that
is booting up in parallel.

For example - if remoteproc A and B crash one after the other, after
remoteproc A boots up, if the remoteproc A tries to get the state of
remoteproc B before the sysmon subdevice for B is invoked but after
the ssr subdevice of B has been invoked, clients on remoteproc A
might get confused when the sysmon notification indicates a different
state.

Signed-off-by: Siddharth Gupta <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
---
drivers/remoteproc/qcom_sysmon.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index a9f04dd83ab6..57dde2a69b9d 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -512,10 +512,12 @@ static int sysmon_start(struct rproc_subdev *subdev)

mutex_lock(&sysmon_lock);
list_for_each_entry(target, &sysmon_list, node) {
- if (target == sysmon)
+ mutex_lock(&target->state_lock);
+ if (target == sysmon || target->state != SSCTL_SSR_EVENT_AFTER_POWERUP) {
+ mutex_unlock(&target->state_lock);
continue;
+ }

- mutex_lock(&target->state_lock);
event.subsys_name = target->name;
event.ssr_event = target->state;

--
2.7.4


2022-07-06 13:34:37

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [V3 7/7] remoteproc: sysmon: Send sysmon state only for running rprocs



On 5.07.2022 14:08, Sibi Sankar wrote:
> From: Siddharth Gupta <[email protected]>
>
> When a new remoteproc boots up, send the sysmon state notification
> of only running remoteprocs. Sending state of remoteprocs booting
> up in parallel can cause a race between SSR clients of the remoteproc
> that is booting up and the sysmon notification for the same remoteproc,
> resulting in an inconsistency between which state the remoteproc that
> is booting up in parallel.
>
> For example - if remoteproc A and B crash one after the other, after
> remoteproc A boots up, if the remoteproc A tries to get the state of
> remoteproc B before the sysmon subdevice for B is invoked but after
> the ssr subdevice of B has been invoked, clients on remoteproc A
> might get confused when the sysmon notification indicates a different
> state.
>
> Signed-off-by: Siddharth Gupta <[email protected]>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad
> drivers/remoteproc/qcom_sysmon.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
> index a9f04dd83ab6..57dde2a69b9d 100644
> --- a/drivers/remoteproc/qcom_sysmon.c
> +++ b/drivers/remoteproc/qcom_sysmon.c
> @@ -512,10 +512,12 @@ static int sysmon_start(struct rproc_subdev *subdev)
>
> mutex_lock(&sysmon_lock);
> list_for_each_entry(target, &sysmon_list, node) {
> - if (target == sysmon)
> + mutex_lock(&target->state_lock);
> + if (target == sysmon || target->state != SSCTL_SSR_EVENT_AFTER_POWERUP) {
> + mutex_unlock(&target->state_lock);
> continue;
> + }
>
> - mutex_lock(&target->state_lock);
> event.subsys_name = target->name;
> event.ssr_event = target->state;
>