From: Tony Lindgren <tony@atomide.com>
To: Dave Gerlach <d-gerlach@ti.com>
Cc: Santosh Shilimkar <ssantosh@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
Suman Anna <s-anna@ti.com>
Subject: Re: [PATCH] soc: ti: wkup_m3_ipc: Fix race condition with rproc_boot
Date: Tue, 17 Dec 2019 10:25:34 -0800 [thread overview]
Message-ID: <20191217182534.GD35479@atomide.com> (raw)
In-Reply-To: <20191212040314.14753-1-d-gerlach@ti.com>
Hi,
* Dave Gerlach <d-gerlach@ti.com> [191211 20:02]:
> Any user of wkup_m3_ipc calls wkup_m3_ipc_get to get a handle and this
> checks the value of the static variable m3_ipc_state to see if the
> wkup_m3 is ready. Currently this is populated during probe before
> rproc_boot has been called, meaning there is a window of time that
> wkup_m3_ipc_get can return a valid handle but the wkup_m3 itself is not
> ready, leading to invalid IPC calls to the wkup_m3 and system
> instability.
>
> To avoid this, move the population of the m3_ipc_state variable until
> after rproc_boot has succeeded to guarantee a valid and usable handle
> is always returned.
Santosh, do you want me to pick this one into my fixes branch?
Regards,
Tony
> Reported-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
> drivers/soc/ti/wkup_m3_ipc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
> index 378369d9364a..e9ece45d7a33 100644
> --- a/drivers/soc/ti/wkup_m3_ipc.c
> +++ b/drivers/soc/ti/wkup_m3_ipc.c
> @@ -419,6 +419,8 @@ static void wkup_m3_rproc_boot_thread(struct wkup_m3_ipc *m3_ipc)
> ret = rproc_boot(m3_ipc->rproc);
> if (ret)
> dev_err(dev, "rproc_boot failed\n");
> + else
> + m3_ipc_state = m3_ipc;
>
> do_exit(0);
> }
> @@ -505,8 +507,6 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
> goto err_put_rproc;
> }
>
> - m3_ipc_state = m3_ipc;
> -
> return 0;
>
> err_put_rproc:
> --
> 2.20.1
>
WARNING: multiple messages have this Message-ID (diff)
From: Tony Lindgren <tony@atomide.com>
To: Dave Gerlach <d-gerlach@ti.com>
Cc: Suman Anna <s-anna@ti.com>,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Santosh Shilimkar <ssantosh@kernel.org>
Subject: Re: [PATCH] soc: ti: wkup_m3_ipc: Fix race condition with rproc_boot
Date: Tue, 17 Dec 2019 10:25:34 -0800 [thread overview]
Message-ID: <20191217182534.GD35479@atomide.com> (raw)
In-Reply-To: <20191212040314.14753-1-d-gerlach@ti.com>
Hi,
* Dave Gerlach <d-gerlach@ti.com> [191211 20:02]:
> Any user of wkup_m3_ipc calls wkup_m3_ipc_get to get a handle and this
> checks the value of the static variable m3_ipc_state to see if the
> wkup_m3 is ready. Currently this is populated during probe before
> rproc_boot has been called, meaning there is a window of time that
> wkup_m3_ipc_get can return a valid handle but the wkup_m3 itself is not
> ready, leading to invalid IPC calls to the wkup_m3 and system
> instability.
>
> To avoid this, move the population of the m3_ipc_state variable until
> after rproc_boot has succeeded to guarantee a valid and usable handle
> is always returned.
Santosh, do you want me to pick this one into my fixes branch?
Regards,
Tony
> Reported-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
> drivers/soc/ti/wkup_m3_ipc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
> index 378369d9364a..e9ece45d7a33 100644
> --- a/drivers/soc/ti/wkup_m3_ipc.c
> +++ b/drivers/soc/ti/wkup_m3_ipc.c
> @@ -419,6 +419,8 @@ static void wkup_m3_rproc_boot_thread(struct wkup_m3_ipc *m3_ipc)
> ret = rproc_boot(m3_ipc->rproc);
> if (ret)
> dev_err(dev, "rproc_boot failed\n");
> + else
> + m3_ipc_state = m3_ipc;
>
> do_exit(0);
> }
> @@ -505,8 +507,6 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
> goto err_put_rproc;
> }
>
> - m3_ipc_state = m3_ipc;
> -
> return 0;
>
> err_put_rproc:
> --
> 2.20.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-12-17 18:25 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-12 4:03 [PATCH] soc: ti: wkup_m3_ipc: Fix race condition with rproc_boot Dave Gerlach
2019-12-12 4:03 ` Dave Gerlach
2019-12-12 4:03 ` Dave Gerlach
2019-12-17 18:25 ` Tony Lindgren [this message]
2019-12-17 18:25 ` Tony Lindgren
2019-12-17 18:32 ` santosh.shilimkar
2019-12-17 18:32 ` santosh.shilimkar
2019-12-17 18:35 ` Tony Lindgren
2019-12-17 18:35 ` Tony Lindgren
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=20191217182534.GD35479@atomide.com \
--to=tony@atomide.com \
--cc=d-gerlach@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=s-anna@ti.com \
--cc=ssantosh@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.