lkml.org 
[lkml]   [2016]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 06/22] media: Media Controller non-locking __media_entity_pipeline_start/stop()
Em Wed, 03 Feb 2016 21:03:38 -0700
Shuah Khan <shuahkh@osg.samsung.com> escreveu:

> Add non-locking __media_entity_pipeline_start/stop() interfaces
> to be called from code paths that hold the graph_mutex. For this
> change, the media_entity_pipeline_start() routine is renamed to
> __media_entity_pipeline_start() minus the graph_mutex lock and
> unlock. media_entity_pipeline_start() now calls the non-locking
> __media_entity_pipeline_start() holding the graph_lock. The stop
> interface, media_entity_pipeline_stop() routine is renamed to
> __media_entity_pipeline_stop() minus the graph_mutex lock and
> unlock. media_entity_pipeline_stop() now calls the non-locking
> __media_entity_pipeline_stop() holding the graph_lock.

Please write a better patch descriptions. The above could easily
be split into 3 or 4 paragraphs, with would make it easier to
read.

Also, too much details, written on a spaghetti style;)

It is obvious by anyone that looked at the patch that
"media_entity_pipeline_start() routine is renamed to
__media_entity_pipeline_start() minus the graph_mutex lock and
unlock."

I read exactly the same thing at the C code, *and* at the
subject of the e-mail. Patch subject/description should tell
what, why and how (but you should not repeat what can easily
be inferred from looking at the diff).

I would describe it as:

Currently, media pipeline graph traversal routines take
the graph_mutex lock.

While this works for the current code, we'll need
unlocked versions of them because
(some explanation *why* you need it, with is the main
thing that should be at the description).

So, add non-locking media_entity_pipeline_start/stop()
variations.

>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> drivers/media/media-entity.c | 34 ++++++++++++++++++++++++----------
> include/media/media-entity.h | 12 ++++++++++++
> 2 files changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 9b4d712..b78e4c2 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -365,8 +365,8 @@ EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
> * Pipeline management
> */
>
> -__must_check int media_entity_pipeline_start(struct media_entity *entity,
> - struct media_pipeline *pipe)
> +__must_check int __media_entity_pipeline_start(struct media_entity *entity,
> + struct media_pipeline *pipe)
> {
> struct media_device *mdev = entity->graph_obj.mdev;
> struct media_entity_graph *graph = &pipe->graph;
> @@ -374,8 +374,6 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
> struct media_link *link;
> int ret;
>
> - mutex_lock(&mdev->graph_mutex);
> -
> if (!pipe->streaming_count++) {
> ret = media_entity_graph_walk_init(&pipe->graph, mdev);
> if (ret)
> @@ -456,8 +454,6 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
> }
> }
>
> - mutex_unlock(&mdev->graph_mutex);
> -
> return 0;
>
> error:
> @@ -487,19 +483,28 @@ error_graph_walk_start:
> if (!--pipe->streaming_count)
> media_entity_graph_walk_cleanup(graph);
>
> - mutex_unlock(&mdev->graph_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(__media_entity_pipeline_start);
>
> +__must_check int media_entity_pipeline_start(struct media_entity *entity,
> + struct media_pipeline *pipe)
> +{
> + struct media_device *mdev = entity->graph_obj.mdev;
> + int ret;
> +
> + mutex_lock(&mdev->graph_mutex);
> + ret = __media_entity_pipeline_start(entity, pipe);
> + mutex_unlock(&mdev->graph_mutex);
> return ret;
> }
> EXPORT_SYMBOL_GPL(media_entity_pipeline_start);
>
> -void media_entity_pipeline_stop(struct media_entity *entity)
> +void __media_entity_pipeline_stop(struct media_entity *entity)
> {
> - struct media_device *mdev = entity->graph_obj.mdev;
> struct media_entity_graph *graph = &entity->pipe->graph;
> struct media_pipeline *pipe = entity->pipe;
>
> - mutex_lock(&mdev->graph_mutex);
>
> WARN_ON(!pipe->streaming_count);
> media_entity_graph_walk_start(graph, entity);
> @@ -516,6 +521,15 @@ void media_entity_pipeline_stop(struct media_entity *entity)
> if (!--pipe->streaming_count)
> media_entity_graph_walk_cleanup(graph);
>
> +}
> +EXPORT_SYMBOL_GPL(__media_entity_pipeline_stop);
> +
> +void media_entity_pipeline_stop(struct media_entity *entity)
> +{
> + struct media_device *mdev = entity->graph_obj.mdev;
> +
> + mutex_lock(&mdev->graph_mutex);
> + __media_entity_pipeline_stop(entity);
> mutex_unlock(&mdev->graph_mutex);
> }
> EXPORT_SYMBOL_GPL(media_entity_pipeline_stop);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index fe485d3..c7583db 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -832,6 +832,12 @@ media_entity_graph_walk_next(struct media_entity_graph *graph);
> */
> __must_check int media_entity_pipeline_start(struct media_entity *entity,
> struct media_pipeline *pipe);
> +/**
> + * non-locking __media_entity_pipeline_start() can be called from
> + * code paths that hold the graph_mutex
> + */
> +__must_check int __media_entity_pipeline_start(struct media_entity *entity,
> + struct media_pipeline *pipe);
>
> /**
> * media_entity_pipeline_stop - Mark a pipeline as not streaming
> @@ -848,6 +854,12 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
> void media_entity_pipeline_stop(struct media_entity *entity);
>
> /**
> + * non-locking __media_entity_pipeline_stop() can be called from
> + * code paths that hold the graph_mutex
> + */
> +void __media_entity_pipeline_stop(struct media_entity *entity);
> +
> +/**
> * media_devnode_create() - creates and initializes a device node interface
> *
> * @mdev: pointer to struct &media_device

\
 
 \ /
  Last update: 2016-02-04 10:41    [W:0.203 / U:1.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site