From: Hans Verkuil <hverkuil@xs4all.nl>
To: Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org
Cc: laurent.pinchart@ideasonboard.com, mchehab@osg.samsung.com
Subject: Re: [PATCH v3 4/5] media: Add flags to tell whether to take graph mutex for an IOCTL
Date: Fri, 22 Jul 2016 12:28:34 +0200 [thread overview]
Message-ID: <24575888-0abf-cc2f-d1c0-8053c9c9fd2c@xs4all.nl> (raw)
In-Reply-To: <1469099851-11026-1-git-send-email-sakari.ailus@linux.intel.com>
On 07/21/2016 01:17 PM, Sakari Ailus wrote:
> New IOCTLs (especially for the request API) do not necessarily need the
> graph mutex acquired. Leave this up to the drivers.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks!
Hans
> ---
> drivers/media/media-device.c | 47 ++++++++++++++++++++++++++------------------
> 1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 87d17a0..6dfcc50 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -381,20 +381,25 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
> return 0;
> }
>
> -#define MEDIA_IOC_ARG(__cmd, func, from_user, to_user) \
> - [_IOC_NR(MEDIA_IOC_##__cmd)] = { \
> - .cmd = MEDIA_IOC_##__cmd, \
> +/* Do acquire the graph mutex */
> +#define MEDIA_IOC_FL_GRAPH_MUTEX BIT(0)
> +
> +#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
> + [_IOC_NR(MEDIA_IOC_##__cmd)] = { \
> + .cmd = MEDIA_IOC_##__cmd, \
> .fn = (long (*)(struct media_device *, void *))func, \
> - .arg_from_user = from_user, \
> - .arg_to_user = to_user, \
> + .flags = fl, \
> + .arg_from_user = from_user, \
> + .arg_to_user = to_user, \
> }
>
> -#define MEDIA_IOC(__cmd, func) \
> - MEDIA_IOC_ARG(__cmd, func, copy_arg_from_user, copy_arg_to_user)
> +#define MEDIA_IOC(__cmd, func, fl) \
> + MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
>
> /* the table is indexed by _IOC_NR(cmd) */
> struct media_ioctl_info {
> unsigned int cmd;
> + unsigned short flags;
> long (*fn)(struct media_device *dev, void *arg);
> long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
> long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
> @@ -435,9 +440,13 @@ static long __media_device_ioctl(
> goto out_free;
> }
>
> - mutex_lock(&dev->graph_mutex);
> + if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
> + mutex_lock(&dev->graph_mutex);
> +
> ret = info->fn(dev, karg);
> - mutex_unlock(&dev->graph_mutex);
> +
> + if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
> + mutex_unlock(&dev->graph_mutex);
>
> if (!ret && info->arg_to_user)
> ret = info->arg_to_user(arg, karg, cmd);
> @@ -450,11 +459,11 @@ out_free:
> }
>
> static const struct media_ioctl_info ioctl_info[] = {
> - MEDIA_IOC(DEVICE_INFO, media_device_get_info),
> - MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities),
> - MEDIA_IOC(ENUM_LINKS, media_device_enum_links),
> - MEDIA_IOC(SETUP_LINK, media_device_setup_link),
> - MEDIA_IOC(G_TOPOLOGY, media_device_get_topology),
> + MEDIA_IOC(DEVICE_INFO, media_device_get_info, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> };
>
> static long media_device_ioctl(struct file *filp, unsigned int cmd,
> @@ -497,11 +506,11 @@ static long from_user_enum_links32(void *karg, void __user *uarg,
> #define MEDIA_IOC_ENUM_LINKS32 _IOWR('|', 0x02, struct media_links_enum32)
>
> static const struct media_ioctl_info compat_ioctl_info[] = {
> - MEDIA_IOC(DEVICE_INFO, media_device_get_info),
> - MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities),
> - MEDIA_IOC_ARG(ENUM_LINKS32, media_device_enum_links, from_user_enum_links32, NULL),
> - MEDIA_IOC(SETUP_LINK, media_device_setup_link),
> - MEDIA_IOC(G_TOPOLOGY, media_device_get_topology),
> + MEDIA_IOC(DEVICE_INFO, media_device_get_info, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC_ARG(ENUM_LINKS32, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX, from_user_enum_links32, NULL),
> + MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> };
>
> static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
>
prev parent reply other threads:[~2016-07-22 10:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-21 11:14 [PATCH v3 0/5] Refactor media IOCTL handling, add variable length arguments Sakari Ailus
2016-07-21 11:14 ` [PATCH v3 1/5] media: Determine early whether an IOCTL is supported Sakari Ailus
2016-07-22 10:27 ` Hans Verkuil
2016-07-22 11:04 ` Sakari Ailus
2016-07-21 11:14 ` [PATCH v3 2/5] media: Unify IOCTL handler calling Sakari Ailus
2016-07-22 10:28 ` Hans Verkuil
2016-07-21 11:14 ` [PATCH v3 3/5] media: Refactor copying IOCTL arguments from and to user space Sakari Ailus
2016-07-22 10:28 ` Hans Verkuil
2016-07-21 11:17 ` [PATCH v3 4/5] media: Add flags to tell whether to take graph mutex for an IOCTL Sakari Ailus
2016-07-21 11:17 ` [PATCH v3 5/5] media: Support variable size IOCTL arguments Sakari Ailus
2016-07-22 10:36 ` Hans Verkuil
2016-08-11 20:38 ` Sakari Ailus
2016-07-22 10:28 ` Hans Verkuil [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=24575888-0abf-cc2f-d1c0-8053c9c9fd2c@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.