Hi Wentong,
Thanks for the patchset.
On Mon, Jun 03, 2024 at 04:26:13PM +0800, Wentong Wu wrote:
> There're possibilities that privacy status change notification happens
> in the middle of the ongoing mei command which already takes the command
> lock, but v4l2_ctrl_s_ctrl() would also need the same lock prior to this
> patch, so this may results in circular locking problem. This patch adds
> one dedicated lock for v4l2 control handler to avoid described issue.
Before this patch, wouldn't the ongoing MEI command simply complete before
v4l2_ctrl_s_ctrl() could proceed?
>
> Reported-by: Hao Yao <hao.yao@xxxxxxxxx>
> Signed-off-by: Wentong Wu <wentong.wu@xxxxxxxxx>
> Tested-by: Jason Chen <jason.z.chen@xxxxxxxxx>
> ---
> drivers/media/pci/intel/ivsc/mei_csi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c b/drivers/media/pci/intel/ivsc/mei_csi.c
> index 004ebab0b814..d6ba0d9efca1 100644
> --- a/drivers/media/pci/intel/ivsc/mei_csi.c
> +++ b/drivers/media/pci/intel/ivsc/mei_csi.c
> @@ -126,6 +126,8 @@ struct mei_csi {
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_ctrl *freq_ctrl;
> struct v4l2_ctrl *privacy_ctrl;
> + /* lock for v4l2 controls */
> + struct mutex ctrl_lock;
> unsigned int remote_pad;
> /* start streaming or not */
> int streaming;
> @@ -563,11 +565,13 @@ static int mei_csi_init_controls(struct mei_csi *csi)
> u32 max;
> int ret;
>
> + mutex_init(&csi->ctrl_lock);
> +
> ret = v4l2_ctrl_handler_init(&csi->ctrl_handler, 2);
> if (ret)
> return ret;
>
> - csi->ctrl_handler.lock = &csi->lock;
> + csi->ctrl_handler.lock = &csi->ctrl_lock;
>
> max = ARRAY_SIZE(link_freq_menu_items) - 1;
> csi->freq_ctrl = v4l2_ctrl_new_int_menu(&csi->ctrl_handler,
> @@ -756,6 +760,7 @@ static int mei_csi_probe(struct mei_cl_device *cldev,
>
> err_ctrl_handler:
> v4l2_ctrl_handler_free(&csi->ctrl_handler);
> + mutex_destroy(&csi->ctrl_lock);
> v4l2_async_nf_unregister(&csi->notifier);
> v4l2_async_nf_cleanup(&csi->notifier);
>
> @@ -775,6 +780,7 @@ static void mei_csi_remove(struct mei_cl_device *cldev)
> v4l2_async_nf_unregister(&csi->notifier);
> v4l2_async_nf_cleanup(&csi->notifier);
> v4l2_ctrl_handler_free(&csi->ctrl_handler);
> + mutex_destroy(&csi->ctrl_lock);
> v4l2_async_unregister_subdev(&csi->subdev);
> v4l2_subdev_cleanup(&csi->subdev);
> media_entity_cleanup(&csi->subdev.entity);
--
Kind regards,
Sakari Ailus
