* [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails
@ 2020-01-13 18:59 Dafna Hirschfeld
2020-01-13 18:59 ` [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error Dafna Hirschfeld
2020-01-13 22:59 ` [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Helen Koike
0 siblings, 2 replies; 4+ messages in thread
From: Dafna Hirschfeld @ 2020-01-13 18:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, skhan, hverkuil, kernel, dafna3
In case kthread_run fails, the vimc subdevices
should be notified that streaming stopped so they can
release the memory for the streaming. Also, kthread should be
set to NULL.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/media/platform/vimc/vimc-streamer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 5c5d3c068398..47a7470e581e 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -207,8 +207,13 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
stream->kthread = kthread_run(vimc_streamer_thread, stream,
"vimc-streamer thread");
- if (IS_ERR(stream->kthread))
- return PTR_ERR(stream->kthread);
+ if (IS_ERR(stream->kthread)) {
+ ret = PTR_ERR(stream->kthread);
+ dev_err(ved->dev, "kthread_run failed with %d\n", ret);
+ vimc_streamer_pipeline_terminate(stream);
+ stream->kthread = NULL;
+ return ret;
+ }
} else {
if (!stream->kthread)
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error
2020-01-13 18:59 [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Dafna Hirschfeld
@ 2020-01-13 18:59 ` Dafna Hirschfeld
2020-01-13 23:09 ` Helen Koike
2020-01-13 22:59 ` [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Helen Koike
1 sibling, 1 reply; 4+ messages in thread
From: Dafna Hirschfeld @ 2020-01-13 18:59 UTC (permalink / raw)
To: linux-media
Cc: dafna.hirschfeld, helen.koike, ezequiel, skhan, hverkuil, kernel, dafna3
Ignore errors returned from kthread_stop since the
vimc subdevices should still be notified that
streaming stopped so they can release the memory for
the streaming, and also kthread should be set to NULL.
kthread_stop can return -EINTR in case the thread
did not yet ran. This can happen if userspace calls
streamon and streamoff right after.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/media/platform/vimc/vimc-streamer.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index cd6b55433c9e..5c5d3c068398 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -216,8 +216,7 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
ret = kthread_stop(stream->kthread);
if (ret)
- return ret;
-
+ dev_warn(ved->dev, "kthread_stop returned '%d'\n", ret);
stream->kthread = NULL;
vimc_streamer_pipeline_terminate(stream);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails
2020-01-13 18:59 [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Dafna Hirschfeld
2020-01-13 18:59 ` [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error Dafna Hirschfeld
@ 2020-01-13 22:59 ` Helen Koike
1 sibling, 0 replies; 4+ messages in thread
From: Helen Koike @ 2020-01-13 22:59 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media; +Cc: ezequiel, skhan, hverkuil, kernel, dafna3
On 1/13/20 4:59 PM, Dafna Hirschfeld wrote:
> In case kthread_run fails, the vimc subdevices
> should be notified that streaming stopped so they can
> release the memory for the streaming. Also, kthread should be
> set to NULL.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/media/platform/vimc/vimc-streamer.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index 5c5d3c068398..47a7470e581e 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -207,8 +207,13 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
> stream->kthread = kthread_run(vimc_streamer_thread, stream,
> "vimc-streamer thread");
>
> - if (IS_ERR(stream->kthread))
> - return PTR_ERR(stream->kthread);
> + if (IS_ERR(stream->kthread)) {
> + ret = PTR_ERR(stream->kthread);
> + dev_err(ved->dev, "kthread_run failed with %d\n", ret);
> + vimc_streamer_pipeline_terminate(stream);
> + stream->kthread = NULL;
> + return ret;
> + }
>
> } else {
> if (!stream->kthread)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error
2020-01-13 18:59 ` [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error Dafna Hirschfeld
@ 2020-01-13 23:09 ` Helen Koike
0 siblings, 0 replies; 4+ messages in thread
From: Helen Koike @ 2020-01-13 23:09 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media; +Cc: ezequiel, skhan, hverkuil, kernel, dafna3
On 1/13/20 4:59 PM, Dafna Hirschfeld wrote:
> Ignore errors returned from kthread_stop since the
> vimc subdevices should still be notified that
> streaming stopped so they can release the memory for
> the streaming, and also kthread should be set to NULL.
> kthread_stop can return -EINTR in case the thread
> did not yet ran. This can happen if userspace calls
> streamon and streamoff right after.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/media/platform/vimc/vimc-streamer.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index cd6b55433c9e..5c5d3c068398 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -216,8 +216,7 @@ int vimc_streamer_s_stream(struct vimc_stream *stream,
>
> ret = kthread_stop(stream->kthread);
Could you add a comment here too? To make it clean in the code why we are ignoring this error?
> if (ret)
> - return ret;
> -
No need to remove this line.
> + dev_warn(ved->dev, "kthread_stop returned '%d'\n", ret);
> stream->kthread = NULL;
>
> vimc_streamer_pipeline_terminate(stream);
>
Thanks
Helen
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-13 23:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13 18:59 [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Dafna Hirschfeld
2020-01-13 18:59 ` [PATCH] media: vimc: streamer: if kthread_stop fails, ignore the error Dafna Hirschfeld
2020-01-13 23:09 ` Helen Koike
2020-01-13 22:59 ` [PATCH] media: vimc: streamer: fix memory leak in vimc subdevs if kthread_run fails Helen Koike
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).