diff options
author | Hans Verkuil <hverkuil-cisco@xs4all.nl> | 2019-03-01 06:11:27 -0500 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab+samsung@kernel.org> | 2019-03-19 13:29:37 -0400 |
commit | 218bf10e39ed5fb22a48dee40bfd2bbcb91693ba (patch) | |
tree | fae09b667509e0ed99447119c42b359186350360 | |
parent | 0e43734d4c46e156785bb1d2acc5b3c10b7d5dd5 (diff) |
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: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
-rw-r--r-- | drivers/media/media-entity.c | 28 | ||||
-rw-r--r-- | drivers/media/v4l2-core/v4l2-subdev.c | 22 | ||||
-rw-r--r-- | include/media/media-entity.h | 24 | ||||
-rw-r--r-- | include/media/v4l2-subdev.h | 2 |
4 files changed, 11 insertions, 65 deletions
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c index 0b1cb3559140..dd859d7b235a 100644 --- a/drivers/media/media-entity.c +++ b/drivers/media/media-entity.c | |||
@@ -17,7 +17,6 @@ | |||
17 | */ | 17 | */ |
18 | 18 | ||
19 | #include <linux/bitmap.h> | 19 | #include <linux/bitmap.h> |
20 | #include <linux/module.h> | ||
21 | #include <linux/property.h> | 20 | #include <linux/property.h> |
22 | #include <linux/slab.h> | 21 | #include <linux/slab.h> |
23 | #include <media/media-entity.h> | 22 | #include <media/media-entity.h> |
@@ -588,33 +587,6 @@ void media_pipeline_stop(struct media_entity *entity) | |||
588 | EXPORT_SYMBOL_GPL(media_pipeline_stop); | 587 | EXPORT_SYMBOL_GPL(media_pipeline_stop); |
589 | 588 | ||
590 | /* ----------------------------------------------------------------------------- | 589 | /* ----------------------------------------------------------------------------- |
591 | * Module use count | ||
592 | */ | ||
593 | |||
594 | struct media_entity *media_entity_get(struct media_entity *entity) | ||
595 | { | ||
596 | if (entity == NULL) | ||
597 | return NULL; | ||
598 | |||
599 | if (entity->graph_obj.mdev->dev && | ||
600 | !try_module_get(entity->graph_obj.mdev->dev->driver->owner)) | ||
601 | return NULL; | ||
602 | |||
603 | return entity; | ||
604 | } | ||
605 | EXPORT_SYMBOL_GPL(media_entity_get); | ||
606 | |||
607 | void media_entity_put(struct media_entity *entity) | ||
608 | { | ||
609 | if (entity == NULL) | ||
610 | return; | ||
611 | |||
612 | if (entity->graph_obj.mdev->dev) | ||
613 | module_put(entity->graph_obj.mdev->dev->driver->owner); | ||
614 | } | ||
615 | EXPORT_SYMBOL_GPL(media_entity_put); | ||
616 | |||
617 | /* ----------------------------------------------------------------------------- | ||
618 | * Links management | 590 | * Links management |
619 | */ | 591 | */ |
620 | 592 | ||
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index f5f0d71ec745..d75815ab0d7b 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c | |||
@@ -18,6 +18,7 @@ | |||
18 | 18 | ||
19 | #include <linux/ioctl.h> | 19 | #include <linux/ioctl.h> |
20 | #include <linux/mm.h> | 20 | #include <linux/mm.h> |
21 | #include <linux/module.h> | ||
21 | #include <linux/slab.h> | 22 | #include <linux/slab.h> |
22 | #include <linux/types.h> | 23 | #include <linux/types.h> |
23 | #include <linux/videodev2.h> | 24 | #include <linux/videodev2.h> |
@@ -54,9 +55,6 @@ static int subdev_open(struct file *file) | |||
54 | struct video_device *vdev = video_devdata(file); | 55 | struct video_device *vdev = video_devdata(file); |
55 | struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); | 56 | struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); |
56 | struct v4l2_subdev_fh *subdev_fh; | 57 | struct v4l2_subdev_fh *subdev_fh; |
57 | #if defined(CONFIG_MEDIA_CONTROLLER) | ||
58 | struct media_entity *entity = NULL; | ||
59 | #endif | ||
60 | int ret; | 58 | int ret; |
61 | 59 | ||
62 | subdev_fh = kzalloc(sizeof(*subdev_fh), GFP_KERNEL); | 60 | subdev_fh = kzalloc(sizeof(*subdev_fh), GFP_KERNEL); |
@@ -73,12 +71,15 @@ static int subdev_open(struct file *file) | |||
73 | v4l2_fh_add(&subdev_fh->vfh); | 71 | v4l2_fh_add(&subdev_fh->vfh); |
74 | file->private_data = &subdev_fh->vfh; | 72 | file->private_data = &subdev_fh->vfh; |
75 | #if defined(CONFIG_MEDIA_CONTROLLER) | 73 | #if defined(CONFIG_MEDIA_CONTROLLER) |
76 | if (sd->v4l2_dev->mdev) { | 74 | if (sd->v4l2_dev->mdev && sd->entity.graph_obj.mdev->dev) { |
77 | entity = media_entity_get(&sd->entity); | 75 | struct module *owner; |
78 | if (!entity) { | 76 | |
77 | owner = sd->entity.graph_obj.mdev->dev->driver->owner; | ||
78 | if (!try_module_get(owner)) { | ||
79 | ret = -EBUSY; | 79 | ret = -EBUSY; |
80 | goto err; | 80 | goto err; |
81 | } | 81 | } |
82 | subdev_fh->owner = owner; | ||
82 | } | 83 | } |
83 | #endif | 84 | #endif |
84 | 85 | ||
@@ -91,9 +92,7 @@ static int subdev_open(struct file *file) | |||
91 | return 0; | 92 | return 0; |
92 | 93 | ||
93 | err: | 94 | err: |
94 | #if defined(CONFIG_MEDIA_CONTROLLER) | 95 | module_put(subdev_fh->owner); |
95 | media_entity_put(entity); | ||
96 | #endif | ||
97 | v4l2_fh_del(&subdev_fh->vfh); | 96 | v4l2_fh_del(&subdev_fh->vfh); |
98 | v4l2_fh_exit(&subdev_fh->vfh); | 97 | v4l2_fh_exit(&subdev_fh->vfh); |
99 | subdev_fh_free(subdev_fh); | 98 | subdev_fh_free(subdev_fh); |
@@ -111,10 +110,7 @@ static int subdev_close(struct file *file) | |||
111 | 110 | ||
112 | if (sd->internal_ops && sd->internal_ops->close) | 111 | if (sd->internal_ops && sd->internal_ops->close) |
113 | sd->internal_ops->close(sd, subdev_fh); | 112 | sd->internal_ops->close(sd, subdev_fh); |
114 | #if defined(CONFIG_MEDIA_CONTROLLER) | 113 | module_put(subdev_fh->owner); |
115 | if (sd->v4l2_dev->mdev) | ||
116 | media_entity_put(&sd->entity); | ||
117 | #endif | ||
118 | v4l2_fh_del(vfh); | 114 | v4l2_fh_del(vfh); |
119 | v4l2_fh_exit(vfh); | 115 | v4l2_fh_exit(vfh); |
120 | subdev_fh_free(subdev_fh); | 116 | subdev_fh_free(subdev_fh); |
diff --git a/include/media/media-entity.h b/include/media/media-entity.h index e5f6960d92f6..3cbad42e3693 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h | |||
@@ -865,19 +865,6 @@ struct media_link *media_entity_find_link(struct media_pad *source, | |||
865 | struct media_pad *media_entity_remote_pad(const struct media_pad *pad); | 865 | struct media_pad *media_entity_remote_pad(const struct media_pad *pad); |
866 | 866 | ||
867 | /** | 867 | /** |
868 | * media_entity_get - Get a reference to the parent module | ||
869 | * | ||
870 | * @entity: The entity | ||
871 | * | ||
872 | * Get a reference to the parent media device module. | ||
873 | * | ||
874 | * The function will return immediately if @entity is %NULL. | ||
875 | * | ||
876 | * Return: returns a pointer to the entity on success or %NULL on failure. | ||
877 | */ | ||
878 | struct media_entity *media_entity_get(struct media_entity *entity); | ||
879 | |||
880 | /** | ||
881 | * media_entity_get_fwnode_pad - Get pad number from fwnode | 868 | * media_entity_get_fwnode_pad - Get pad number from fwnode |
882 | * | 869 | * |
883 | * @entity: The entity | 870 | * @entity: The entity |
@@ -917,17 +904,6 @@ __must_check int media_graph_walk_init( | |||
917 | void media_graph_walk_cleanup(struct media_graph *graph); | 904 | void media_graph_walk_cleanup(struct media_graph *graph); |
918 | 905 | ||
919 | /** | 906 | /** |
920 | * media_entity_put - Release the reference to the parent module | ||
921 | * | ||
922 | * @entity: The entity | ||
923 | * | ||
924 | * Release the reference count acquired by media_entity_get(). | ||
925 | * | ||
926 | * The function will return immediately if @entity is %NULL. | ||
927 | */ | ||
928 | void media_entity_put(struct media_entity *entity); | ||
929 | |||
930 | /** | ||
931 | * media_graph_walk_start - Start walking the media graph at a | 907 | * media_graph_walk_start - Start walking the media graph at a |
932 | * given entity | 908 | * given entity |
933 | * | 909 | * |
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index e807aa96ed3b..a7fa5b80915a 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h | |||
@@ -910,9 +910,11 @@ struct v4l2_subdev { | |||
910 | * | 910 | * |
911 | * @vfh: pointer to &struct v4l2_fh | 911 | * @vfh: pointer to &struct v4l2_fh |
912 | * @pad: pointer to &struct v4l2_subdev_pad_config | 912 | * @pad: pointer to &struct v4l2_subdev_pad_config |
913 | * @owner: module pointer to the owner of this file handle | ||
913 | */ | 914 | */ |
914 | struct v4l2_subdev_fh { | 915 | struct v4l2_subdev_fh { |
915 | struct v4l2_fh vfh; | 916 | struct v4l2_fh vfh; |
917 | struct module *owner; | ||
916 | #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) | 918 | #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) |
917 | struct v4l2_subdev_pad_config *pad; | 919 | struct v4l2_subdev_pad_config *pad; |
918 | #endif | 920 | #endif |