Commit 218bf10e authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab
Browse files

media: v4l2-subdev: handle module refcounting here 



The module ownership refcounting was done in media_entity_get/put,
but that was very confusing and it did not work either in case an
application had a v4l-subdevX device open and the module was
unbound. When the v4l-subdevX device was closed the media_entity_put
was never called and the module refcount was left one too high, making
it impossible to unload it.

Since v4l2-subdev.c was the only place where media_entity_get/put was
called, just move the functionality to v4l2-subdev.c and drop those
confusing entity functions.

Store the module in subdev_fh so module_put no longer depends on
the media_entity struct.

Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+samsung@kernel.org>
parent 0e43734d

drivers/media/media-entity.c

+0 −28
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
 */



#include <linux/bitmap.h>

#include <linux/module.h>

#include <linux/property.h>

#include <linux/slab.h>

#include <media/media-entity.h>
@@ -587,33 +586,6 @@ void media_pipeline_stop(struct media_entity *entity) 
}

EXPORT_SYMBOL_GPL(media_pipeline_stop);



/* -----------------------------------------------------------------------------

 * Module use count

 */



struct media_entity *media_entity_get(struct media_entity *entity)

{

	if (entity == NULL)

		return NULL;



	if (entity->graph_obj.mdev->dev &&

	    !try_module_get(entity->graph_obj.mdev->dev->driver->owner))

		return NULL;



	return entity;

}

EXPORT_SYMBOL_GPL(media_entity_get);



void media_entity_put(struct media_entity *entity)

{

	if (entity == NULL)

		return;



	if (entity->graph_obj.mdev->dev)

		module_put(entity->graph_obj.mdev->dev->driver->owner);

}

EXPORT_SYMBOL_GPL(media_entity_put);



/* -----------------------------------------------------------------------------

 * Links management

 */

drivers/media/v4l2-core/v4l2-subdev.c

+9 −13
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 


#include <linux/ioctl.h>

#include <linux/mm.h>

#include <linux/module.h>

#include <linux/slab.h>

#include <linux/types.h>

#include <linux/videodev2.h>
@@ -54,9 +55,6 @@ static int subdev_open(struct file *file) 
	struct video_device *vdev = video_devdata(file);

	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);

	struct v4l2_subdev_fh *subdev_fh;

#if defined(CONFIG_MEDIA_CONTROLLER)

	struct media_entity *entity = NULL;

#endif

	int ret;



	subdev_fh = kzalloc(sizeof(*subdev_fh), GFP_KERNEL);
@@ -73,12 +71,15 @@ static int subdev_open(struct file *file) 
	v4l2_fh_add(&subdev_fh->vfh);

	file->private_data = &subdev_fh->vfh;

#if defined(CONFIG_MEDIA_CONTROLLER)

	if (sd->v4l2_dev->mdev) {

		entity = media_entity_get(&sd->entity);

		if (!entity) {

	if (sd->v4l2_dev->mdev && sd->entity.graph_obj.mdev->dev) {

		struct module *owner;



		owner = sd->entity.graph_obj.mdev->dev->driver->owner;

		if (!try_module_get(owner)) {

			ret = -EBUSY;

			goto err;

		}

		subdev_fh->owner = owner;

	}

#endif
@@ -91,9 +92,7 @@ static int subdev_open(struct file *file) 
	return 0;



err:

#if defined(CONFIG_MEDIA_CONTROLLER)

	media_entity_put(entity);

#endif

	module_put(subdev_fh->owner);

	v4l2_fh_del(&subdev_fh->vfh);

	v4l2_fh_exit(&subdev_fh->vfh);

	subdev_fh_free(subdev_fh);
@@ -111,10 +110,7 @@ static int subdev_close(struct file *file) 


	if (sd->internal_ops && sd->internal_ops->close)

		sd->internal_ops->close(sd, subdev_fh);

#if defined(CONFIG_MEDIA_CONTROLLER)

	if (sd->v4l2_dev->mdev)

		media_entity_put(&sd->entity);

#endif

	module_put(subdev_fh->owner);

	v4l2_fh_del(vfh);

	v4l2_fh_exit(vfh);

	subdev_fh_free(subdev_fh);

include/media/media-entity.h

+0 −24
Original line number Diff line number Diff line
@@ -864,19 +864,6 @@ struct media_link *media_entity_find_link(struct media_pad *source, 
 */

struct media_pad *media_entity_remote_pad(const struct media_pad *pad);



/**

 * media_entity_get - Get a reference to the parent module

 *

 * @entity: The entity

 *

 * Get a reference to the parent media device module.

 *

 * The function will return immediately if @entity is %NULL.

 *

 * Return: returns a pointer to the entity on success or %NULL on failure.

 */

struct media_entity *media_entity_get(struct media_entity *entity);



/**

 * media_entity_get_fwnode_pad - Get pad number from fwnode

 *
@@ -916,17 +903,6 @@ __must_check int media_graph_walk_init( 
 */

void media_graph_walk_cleanup(struct media_graph *graph);



/**

 * media_entity_put - Release the reference to the parent module

 *

 * @entity: The entity

 *

 * Release the reference count acquired by media_entity_get().

 *

 * The function will return immediately if @entity is %NULL.

 */

void media_entity_put(struct media_entity *entity);



/**

 * media_graph_walk_start - Start walking the media graph at a

 *	given entity

include/media/v4l2-subdev.h

+2 −0
Original line number Diff line number Diff line
@@ -910,9 +910,11 @@ struct v4l2_subdev { 
 *

 * @vfh: pointer to &struct v4l2_fh

 * @pad: pointer to &struct v4l2_subdev_pad_config

 * @owner: module pointer to the owner of this file handle

 */

struct v4l2_subdev_fh {

	struct v4l2_fh vfh;

	struct module *owner;

#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)

	struct v4l2_subdev_pad_config *pad;

#endif