Re: [PATCH v2 10/29] media: mc: Delete character device early

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

Thank you for the patch.

On Wed, Dec 20, 2023 at 12:36:54PM +0200, Sakari Ailus wrote:
> The parent of the character device related to the media devnode is the
> media devnode. Thus the character device needs to be released before the
> media devnode's release function. Move it to unregistering of the media
> devnode, which mirrors adding the character device in conjunction with
> registering the media devnode.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Acked-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
>  drivers/media/mc/mc-devnode.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
> index 7e22938dfd81..8bc7450ac144 100644
> --- a/drivers/media/mc/mc-devnode.c
> +++ b/drivers/media/mc/mc-devnode.c
> @@ -51,9 +51,6 @@ static void media_devnode_release(struct device *cd)
>  
>  	mutex_lock(&media_devnode_lock);
>  
> -	/* Delete the cdev on this minor as well */
> -	cdev_del(&devnode->cdev);
> -
>  	/* Mark device node number as free */
>  	clear_bit(devnode->minor, media_devnode_nums);

Should this be moved to media_devnode_unregister() too ? It can be done
in a separate patch.

>  
> @@ -270,6 +267,7 @@ void media_devnode_unregister(struct media_devnode *devnode)
>  	clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
>  	mutex_unlock(&media_devnode_lock);
>  
> +	cdev_del(&devnode->cdev);

I initially wondered if this could raise with the cdev access in
media_open(), as the media_devnode_lock is released just before calling
cdev_dev(), but my understanding is that the dev/open race is properly
handled in the cdev layer.

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

I wonder if a similar change in v4l2-dev.c would be beneficial.

>  	device_unregister(&devnode->dev);
>  }
>  

-- 
Regards,

Laurent Pinchart




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux