| Message ID | 20240909154828.1661627-1-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: platform: video-mux: Fix mutex locking | expand |
Hi Paul, Thank you for the patch. On Mon, Sep 09, 2024 at 05:48:28PM +0200, Paul Elder wrote: > The current order of locking between the driver mutex and the v4l2 > subdev state lock causes a circuluar locking dependency when trying to > set up a link. Fix this. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This being said, I think we should deprecate the video-mux driver and implement a driver that uses the V4L2 subdev internal routing API instead of basing the routing configuration on link setup. Any opinion ? > --- > drivers/media/platform/video-mux.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c > index 31e9e92e723eb..f43849db56800 100644 > --- a/drivers/media/platform/video-mux.c > +++ b/drivers/media/platform/video-mux.c > @@ -52,6 +52,7 @@ static int video_mux_link_setup(struct media_entity *entity, > const struct media_pad *remote, u32 flags) > { > struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); > + struct v4l2_subdev_state *sd_state; > struct video_mux *vmux = v4l2_subdev_to_video_mux(sd); > u16 source_pad = entity->num_pads - 1; > int ret = 0; > @@ -67,10 +68,10 @@ static int video_mux_link_setup(struct media_entity *entity, > remote->entity->name, remote->index, local->entity->name, > local->index, flags & MEDIA_LNK_FL_ENABLED); > > + sd_state = v4l2_subdev_lock_and_get_active_state(sd); > mutex_lock(&vmux->lock); > > if (flags & MEDIA_LNK_FL_ENABLED) { > - struct v4l2_subdev_state *sd_state; > struct v4l2_mbus_framefmt *source_mbusformat; > > if (vmux->active == local->index) > @@ -88,12 +89,10 @@ static int video_mux_link_setup(struct media_entity *entity, > vmux->active = local->index; > > /* Propagate the active format to the source */ > - sd_state = v4l2_subdev_lock_and_get_active_state(sd); > source_mbusformat = v4l2_subdev_state_get_format(sd_state, > source_pad); > *source_mbusformat = *v4l2_subdev_state_get_format(sd_state, > vmux->active); > - v4l2_subdev_unlock_state(sd_state); > } else { > if (vmux->active != local->index) > goto out; > @@ -105,6 +104,7 @@ static int video_mux_link_setup(struct media_entity *entity, > > out: > mutex_unlock(&vmux->lock); > + v4l2_subdev_unlock_state(sd_state); > return ret; > } >
diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c index 31e9e92e723eb..f43849db56800 100644 --- a/drivers/media/platform/video-mux.c +++ b/drivers/media/platform/video-mux.c @@ -52,6 +52,7 @@ static int video_mux_link_setup(struct media_entity *entity, const struct media_pad *remote, u32 flags) { struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); + struct v4l2_subdev_state *sd_state; struct video_mux *vmux = v4l2_subdev_to_video_mux(sd); u16 source_pad = entity->num_pads - 1; int ret = 0; @@ -67,10 +68,10 @@ static int video_mux_link_setup(struct media_entity *entity, remote->entity->name, remote->index, local->entity->name, local->index, flags & MEDIA_LNK_FL_ENABLED); + sd_state = v4l2_subdev_lock_and_get_active_state(sd); mutex_lock(&vmux->lock); if (flags & MEDIA_LNK_FL_ENABLED) { - struct v4l2_subdev_state *sd_state; struct v4l2_mbus_framefmt *source_mbusformat; if (vmux->active == local->index) @@ -88,12 +89,10 @@ static int video_mux_link_setup(struct media_entity *entity, vmux->active = local->index; /* Propagate the active format to the source */ - sd_state = v4l2_subdev_lock_and_get_active_state(sd); source_mbusformat = v4l2_subdev_state_get_format(sd_state, source_pad); *source_mbusformat = *v4l2_subdev_state_get_format(sd_state, vmux->active); - v4l2_subdev_unlock_state(sd_state); } else { if (vmux->active != local->index) goto out; @@ -105,6 +104,7 @@ static int video_mux_link_setup(struct media_entity *entity, out: mutex_unlock(&vmux->lock); + v4l2_subdev_unlock_state(sd_state); return ret; }
The current order of locking between the driver mutex and the v4l2 subdev state lock causes a circuluar locking dependency when trying to set up a link. Fix this. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- drivers/media/platform/video-mux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)