[PATCH RFC v2] DVB: add dvb_generic_nonseekable_open,dvb_generic_unlocked_ioctl, use in firedtv
From: Stefan Richter
Date: Sun Mar 28 2010 - 10:47:32 EST
In order to remove Big Kernel Lock usages from the DVB subsystem,
we need to
- provide .llseek file operations that do not grab the BKL (unlike
fs/read_write.c::default_llseek) or mark files as not seekable,
- provide .unlocked_ioctl file operations.
Add two dvb_generic_ file operations for file interfaces which are not
seekable and, respectively, do not require the BKL in their ioctl
handlers.
Use them in one driver of which I am sure of that these are applicable.
(Affected code paths in firedtv-ci were not runtime-tested since I don't
have a CAM, but the frontend ioctls were of course runtime-tested.)
Notes:
- The dvb-core internal dvb_usercopy() API is changed to match
.unlocked_ioctl() prototypes.
- I suspect that all dvb_generic_open() users really want
nonseekable_open --- then we should simply change dvb_generic_open()
instead of adding dvb_generic_nonseekable_open() --- but I haven't
checked other users of dvb_generic_open whether they require
.llssek mehods other than fs/read_write.c::no_llseek.
Applies to:
drivers/media/dvb/ttpci/av7110.c
drivers/media/dvb/ttpci/av7110_av.c
drivers/media/dvb/ttpci/av7110_ca.c
drivers/media/dvb/dvb-core/dvb_net.c
drivers/media/dvb/dvb-core/dvb_frontend.c
drivers/media/dvb/dvb-core/dvb_ca_en50221.c
- To be done by those who know the code: Check all users of
struct dvb_device.kernel_ioctl whether they really need the BKL.
Convert to .unlocked_ioctl and remove .kernel_ioctl and the
temporarily introduced dvbdev.c::legacy_usercopy().
Applies to:
drivers/media/dvb/ttpci/av7110.c
drivers/media/dvb/ttpci/av7110_av.c
drivers/media/dvb/ttpci/av7110_ca.c
drivers/media/dvb/dvb-core/dvb_frontend.c
Signed-off-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx>
---
Update: Split dvb_usercopy into one that follows the .unlocked_ioctl
prototype form and another one that preserves the old form.
drivers/media/dvb/dvb-core/dmxdev.c | 10 -
drivers/media/dvb/dvb-core/dvb_ca_en50221.c | 6
drivers/media/dvb/dvb-core/dvb_net.c | 5
drivers/media/dvb/dvb-core/dvbdev.c | 190 +++++++++++++-------
drivers/media/dvb/dvb-core/dvbdev.h | 23 +-
drivers/media/dvb/firewire/firedtv-ci.c | 9
6 files changed, 148 insertions(+), 95 deletions(-)
Index: b/drivers/media/dvb/dvb-core/dmxdev.c
===================================================================
--- a/drivers/media/dvb/dvb-core/dmxdev.c
+++ b/drivers/media/dvb/dvb-core/dmxdev.c
@@ -963,8 +963,7 @@ dvb_demux_read(struct file *file, char _
return ret;
}
-static int dvb_demux_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_demux_do_ioctl(struct file *file, unsigned int cmd, void *parg)
{
struct dmxdev_filter *dmxdevfilter = file->private_data;
struct dmxdev *dmxdev = dmxdevfilter->dev;
@@ -1087,7 +1086,7 @@ static int dvb_demux_do_ioctl(struct ino
static int dvb_demux_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
- return dvb_usercopy(inode, file, cmd, arg, dvb_demux_do_ioctl);
+ return dvb_usercopy(file, cmd, arg, dvb_demux_do_ioctl);
}
static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
@@ -1152,8 +1151,7 @@ static struct dvb_device dvbdev_demux =
.fops = &dvb_demux_fops
};
-static int dvb_dvr_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_dvr_do_ioctl(struct file *file, unsigned int cmd, void *parg)
{
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
@@ -1179,7 +1177,7 @@ static int dvb_dvr_do_ioctl(struct inode
static int dvb_dvr_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
- return dvb_usercopy(inode, file, cmd, arg, dvb_dvr_do_ioctl);
+ return dvb_usercopy(file, cmd, arg, dvb_dvr_do_ioctl);
}
static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
Index: b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
===================================================================
--- a/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb/dvb-core/dvb_ca_en50221.c
@@ -1181,8 +1181,8 @@ static int dvb_ca_en50221_thread(void *d
*
* @return 0 on success, <0 on error.
*/
-static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_ca_en50221_io_do_ioctl(struct file *file,
+ unsigned int cmd, void *parg)
{
struct dvb_device *dvbdev = file->private_data;
struct dvb_ca_private *ca = dvbdev->priv;
@@ -1258,7 +1258,7 @@ static int dvb_ca_en50221_io_do_ioctl(st
static int dvb_ca_en50221_io_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
- return dvb_usercopy(inode, file, cmd, arg, dvb_ca_en50221_io_do_ioctl);
+ return dvb_usercopy(file, cmd, arg, dvb_ca_en50221_io_do_ioctl);
}
Index: b/drivers/media/dvb/dvb-core/dvb_net.c
===================================================================
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -1333,8 +1333,7 @@ static int dvb_net_remove_if(struct dvb_
return 0;
}
-static int dvb_net_do_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *parg)
+static long dvb_net_do_ioctl(struct file *file, unsigned int cmd, void *parg)
{
struct dvb_device *dvbdev = file->private_data;
struct dvb_net *dvbnet = dvbdev->priv;
@@ -1438,7 +1437,7 @@ static int dvb_net_do_ioctl(struct inode
static int dvb_net_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
- return dvb_usercopy(inode, file, cmd, arg, dvb_net_do_ioctl);
+ return dvb_usercopy(file, cmd, arg, dvb_net_do_ioctl);
}
static int dvb_net_close(struct inode *inode, struct file *file)
Index: b/drivers/media/dvb/dvb-core/dvbdev.c
===================================================================
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -135,6 +135,18 @@ int dvb_generic_open(struct inode *inode
EXPORT_SYMBOL(dvb_generic_open);
+int dvb_generic_nonseekable_open(struct inode *inode, struct file *file)
+{
+ int retval = dvb_generic_open(inode, file);
+
+ if (retval == 0)
+ retval = nonseekable_open(inode, file);
+
+ return retval;
+}
+EXPORT_SYMBOL(dvb_generic_nonseekable_open);
+
+
int dvb_generic_release(struct inode *inode, struct file *file)
{
struct dvb_device *dvbdev = file->private_data;
@@ -154,6 +166,102 @@ int dvb_generic_release(struct inode *in
EXPORT_SYMBOL(dvb_generic_release);
+#define STATIC_BUFFER_SIZE 128
+
+/* If necessary, swap out static *buf by a slab-allocated buffer */
+static int get_arg(unsigned int cmd, unsigned long arg, void **parg, void **buf)
+{
+ switch (_IOC_DIR(cmd)) {
+ case _IOC_NONE:
+ /*
+ * For this command, the pointer is actually an integer
+ * argument.
+ */
+ *parg = (void *)arg;
+ break;
+ case _IOC_READ: /* some v4l ioctls are marked wrong ... */
+ case _IOC_WRITE:
+ case (_IOC_WRITE | _IOC_READ):
+ if (_IOC_SIZE(cmd) > STATIC_BUFFER_SIZE) {
+ *buf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
+ if (!*buf)
+ return -ENOMEM;
+ }
+ *parg = *buf;
+
+ if (copy_from_user(*parg, (void __user *)arg, _IOC_SIZE(cmd)))
+ return -EFAULT;
+ }
+ return 0;
+}
+
+static int put_arg(unsigned int cmd, unsigned long arg, void *parg)
+{
+ switch (_IOC_DIR(cmd)) {
+ case _IOC_READ:
+ case (_IOC_WRITE | _IOC_READ):
+ if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
+ return -EFAULT;
+ }
+ return 0;
+}
+
+/*
+ * Wrapper around ioctl handlers; copies arguments and results from/ to user.
+ * Could be changed into a "generic_usercopy()" for all kernel subsystems.
+ */
+long dvb_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+ long (*func)(struct file *file, unsigned int cmd, void *arg))
+{
+ char sbuf[STATIC_BUFFER_SIZE];
+ void *parg, *buf = sbuf;
+ long retval;
+
+ retval = get_arg(cmd, arg, &parg, &buf);
+ if (retval < 0)
+ goto out;
+
+ /* call driver */
+ retval = func(file, cmd, parg);
+ if (retval == -ENOIOCTLCMD)
+ retval = -EINVAL;
+ if (retval < 0)
+ goto out;
+
+ retval = put_arg(cmd, arg, parg);
+out:
+ if (buf != sbuf)
+ kfree(buf);
+ return retval;
+}
+
+static int legacy_usercopy(struct inode *inode, struct file *file,
+ unsigned int cmd, unsigned long arg,
+ int (*func)(struct inode *inode, struct file *file,
+ unsigned int cmd, void *arg))
+{
+ char sbuf[STATIC_BUFFER_SIZE];
+ void *parg, *buf = sbuf;
+ int retval;
+
+ retval = get_arg(cmd, arg, &parg, &buf);
+ if (retval < 0)
+ goto out;
+
+ /* call driver */
+ retval = func(inode, file, cmd, parg);
+ if (retval == -ENOIOCTLCMD)
+ retval = -EINVAL;
+ if (retval < 0)
+ goto out;
+
+ retval = put_arg(cmd, arg, parg);
+out:
+ if (buf != sbuf)
+ kfree(buf);
+ return retval;
+}
+
int dvb_generic_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
@@ -165,11 +273,27 @@ int dvb_generic_ioctl(struct inode *inod
if (!dvbdev->kernel_ioctl)
return -EINVAL;
- return dvb_usercopy (inode, file, cmd, arg, dvbdev->kernel_ioctl);
+ return legacy_usercopy(inode, file, cmd, arg, dvbdev->kernel_ioctl);
}
EXPORT_SYMBOL(dvb_generic_ioctl);
+long dvb_generic_unlocked_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ struct dvb_device *dvbdev = file->private_data;
+
+ if (!dvbdev)
+ return -ENODEV;
+
+ if (!dvbdev->unlocked_ioctl)
+ return -EINVAL;
+
+ return dvb_usercopy(file, cmd, arg, dvbdev->unlocked_ioctl);
+}
+EXPORT_SYMBOL(dvb_generic_unlocked_ioctl);
+
+
static int dvbdev_get_free_id (struct dvb_adapter *adap, int type)
{
u32 id = 0;
@@ -372,70 +496,6 @@ int dvb_unregister_adapter(struct dvb_ad
}
EXPORT_SYMBOL(dvb_unregister_adapter);
-/* if the miracle happens and "generic_usercopy()" is included into
- the kernel, then this can vanish. please don't make the mistake and
- define this as video_usercopy(). this will introduce a dependecy
- to the v4l "videodev.o" module, which is unnecessary for some
- cards (ie. the budget dvb-cards don't need the v4l module...) */
-int dvb_usercopy(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg,
- int (*func)(struct inode *inode, struct file *file,
- unsigned int cmd, void *arg))
-{
- char sbuf[128];
- void *mbuf = NULL;
- void *parg = NULL;
- int err = -EINVAL;
-
- /* Copy arguments into temp kernel buffer */
- switch (_IOC_DIR(cmd)) {
- case _IOC_NONE:
- /*
- * For this command, the pointer is actually an integer
- * argument.
- */
- parg = (void *) arg;
- break;
- case _IOC_READ: /* some v4l ioctls are marked wrong ... */
- case _IOC_WRITE:
- case (_IOC_WRITE | _IOC_READ):
- if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
- parg = sbuf;
- } else {
- /* too big to allocate from stack */
- mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
- if (NULL == mbuf)
- return -ENOMEM;
- parg = mbuf;
- }
-
- err = -EFAULT;
- if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd)))
- goto out;
- break;
- }
-
- /* call driver */
- if ((err = func(inode, file, cmd, parg)) == -ENOIOCTLCMD)
- err = -EINVAL;
-
- if (err < 0)
- goto out;
-
- /* Copy results into user buffer */
- switch (_IOC_DIR(cmd))
- {
- case _IOC_READ:
- case (_IOC_WRITE | _IOC_READ):
- if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
- err = -EFAULT;
- break;
- }
-
-out:
- kfree(mbuf);
- return err;
-}
static int dvb_uevent(struct device *dev, struct kobj_uevent_env *env)
{
Index: b/drivers/media/dvb/dvb-core/dvbdev.h
===================================================================
--- a/drivers/media/dvb/dvb-core/dvbdev.h
+++ b/drivers/media/dvb/dvb-core/dvbdev.h
@@ -118,6 +118,7 @@ struct dvb_device {
/* don't really need those !? -- FIXME: use video_usercopy */
int (*kernel_ioctl)(struct inode *inode, struct file *file,
unsigned int cmd, void *arg);
+ long (*unlocked_ioctl)(struct file *file, unsigned int cmd, void *arg);
void *priv;
};
@@ -136,19 +137,15 @@ extern int dvb_register_device (struct d
extern void dvb_unregister_device (struct dvb_device *dvbdev);
-extern int dvb_generic_open (struct inode *inode, struct file *file);
-extern int dvb_generic_release (struct inode *inode, struct file *file);
-extern int dvb_generic_ioctl (struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg);
-
-/* we don't mess with video_usercopy() any more,
-we simply define out own dvb_usercopy(), which will hopefully become
-generic_usercopy() someday... */
-
-extern int dvb_usercopy(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg,
- int (*func)(struct inode *inode, struct file *file,
- unsigned int cmd, void *arg));
+extern int dvb_generic_open(struct inode *inode, struct file *file);
+extern int dvb_generic_nonseekable_open(struct inode *inode, struct file *file);
+extern int dvb_generic_release(struct inode *inode, struct file *file);
+extern int dvb_generic_ioctl(struct inode *inode, struct file *file,
+ unsigned int cmd, unsigned long arg);
+extern long dvb_generic_unlocked_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg);
+extern long dvb_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+ long (*func)(struct file *file, unsigned int cmd, void *arg));
/** generic DVB attach function. */
#ifdef CONFIG_MEDIA_ATTACH
Index: b/drivers/media/dvb/firewire/firedtv-ci.c
===================================================================
--- a/drivers/media/dvb/firewire/firedtv-ci.c
+++ b/drivers/media/dvb/firewire/firedtv-ci.c
@@ -175,8 +175,7 @@ static int fdtv_ca_send_msg(struct fired
return err;
}
-static int fdtv_ca_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, void *arg)
+static long fdtv_ca_ioctl(struct file *file, unsigned int cmd, void *arg)
{
struct dvb_device *dvbdev = file->private_data;
struct firedtv *fdtv = dvbdev->priv;
@@ -217,8 +216,8 @@ static unsigned int fdtv_ca_io_poll(stru
static const struct file_operations fdtv_ca_fops = {
.owner = THIS_MODULE,
- .ioctl = dvb_generic_ioctl,
- .open = dvb_generic_open,
+ .unlocked_ioctl = dvb_generic_unlocked_ioctl,
+ .open = dvb_generic_nonseekable_open,
.release = dvb_generic_release,
.poll = fdtv_ca_io_poll,
};
@@ -228,7 +227,7 @@ static struct dvb_device fdtv_ca = {
.readers = 1,
.writers = 1,
.fops = &fdtv_ca_fops,
- .kernel_ioctl = fdtv_ca_ioctl,
+ .unlocked_ioctl = fdtv_ca_ioctl,
};
int fdtv_ca_register(struct firedtv *fdtv)
--
Stefan Richter
-=====-==-=- --== ===--
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/