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 <stef...@s5r6.in-berlin.de>
---

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-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to