lkml.org 
[lkml]   [2010]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH RFC v2] DVB: add dvb_generic_nonseekable_open, dvb_generic_unlocked_ioctl, use in firedtv
    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@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/


    \
     
     \ /
      Last update: 2010-03-28 16:49    [W:2.216 / U:4.344 seconds]
    ©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site