Re: [PATCH 4/6] staging: push down BKL into ioctl functions

[Frederic Weisbecker]

Hi Greg,


On Tue, Apr 27, 2010 at 12:24:03AM +0200, Arnd Bergmann wrote:
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>  drivers/staging/crystalhd/crystalhd_lnx.c |   13 +++++++++----
>  drivers/staging/dt3155/dt3155_drv.c       |   24 ++++++++++++++++++------
>  drivers/staging/poch/poch.c               |   17 +++++++++++++++--
>  drivers/staging/vme/devices/vme_user.c    |   18 +++++++++++++++---
>  4 files changed, 57 insertions(+), 15 deletions(-)


If you are fine with this patch, I think it would be better if
you get it in your tree.

Thanks.




> 
> diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
> index 54bad65..3291e14 100644
> --- a/drivers/staging/crystalhd/crystalhd_lnx.c
> +++ b/drivers/staging/crystalhd/crystalhd_lnx.c
> @@ -16,6 +16,7 @@
>  ***************************************************************************/
>  
>  #include <linux/version.h>
> +#include <linux/smp_lock.h>
>  #include <linux/slab.h>
>  
>  #include "crystalhd_lnx.h"
> @@ -261,12 +262,12 @@ static int chd_dec_api_cmd(struct crystalhd_adp *adp, unsigned long ua,
>  }
>  
>  /* API interfaces */
> -static int chd_dec_ioctl(struct inode *in, struct file *fd,
> -			 unsigned int cmd, unsigned long ua)
> +static long chd_dec_ioctl(struct file *fd, unsigned int cmd, unsigned long ua)
>  {
>  	struct crystalhd_adp *adp = chd_get_adp();
>  	crystalhd_cmd_proc cproc;
>  	struct crystalhd_user *uc;
> +	int ret;
>  
>  	if (!adp || !fd) {
>  		BCMLOG_ERR("Invalid adp\n");
> @@ -279,13 +280,17 @@ static int chd_dec_ioctl(struct inode *in, struct file *fd,
>  		return -ENODATA;
>  	}
>  
> +	lock_kernel();
>  	cproc = crystalhd_get_cmd_proc(&adp->cmds, cmd, uc);
>  	if (!cproc) {
>  		BCMLOG_ERR("Unhandled command: %d\n", cmd);
> +		unlock_kernel();
>  		return -EINVAL;
>  	}
>  
> -	return chd_dec_api_cmd(adp, ua, uc->uid, cmd, cproc);
> +	ret = chd_dec_api_cmd(adp, ua, uc->uid, cmd, cproc);
> +	unlock_kernel();
> +	return ret;
>  }
>  
>  static int chd_dec_open(struct inode *in, struct file *fd)
> @@ -345,7 +350,7 @@ static int chd_dec_close(struct inode *in, struct file *fd)
>  
>  static const struct file_operations chd_dec_fops = {
>  	.owner   = THIS_MODULE,
> -	.ioctl   = chd_dec_ioctl,
> +	.unlocked_ioctl = chd_dec_ioctl,
>  	.open    = chd_dec_open,
>  	.release = chd_dec_close,
>  };
> diff --git a/drivers/staging/dt3155/dt3155_drv.c b/drivers/staging/dt3155/dt3155_drv.c
> index e2c44ec..7e6095f 100644
> --- a/drivers/staging/dt3155/dt3155_drv.c
> +++ b/drivers/staging/dt3155/dt3155_drv.c
> @@ -63,6 +63,7 @@ extern void printques(int);
>  #include <linux/types.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
> +#include <linux/smp_lock.h>
>  
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
> @@ -835,6 +836,17 @@ static unsigned int dt3155_poll (struct file * filp, poll_table *wait)
>    return 0;
>  }
>  
> +static long
> +dt3155_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	int ret;
> +
> +	lock_kernel();
> +	ret = dt3155_ioctl(file->f_path.dentry->d_inode, file, cmd, arg);
> +	unlock_kernel();
> +
> +	return ret;
> +}
>  
>  /*****************************************************
>   * file operations supported by DT3155 driver
> @@ -842,12 +854,12 @@ static unsigned int dt3155_poll (struct file * filp, poll_table *wait)
>   *  register_chrdev
>   *****************************************************/
>  static struct file_operations dt3155_fops = {
> -  read:		dt3155_read,
> -  ioctl:		dt3155_ioctl,
> -  mmap:		dt3155_mmap,
> -  poll:           dt3155_poll,
> -  open:		dt3155_open,
> -  release:	dt3155_close
> +	.read		= dt3155_read,
> +	.unlocked_ioctl	= dt3155_unlocked_ioctl,
> +	.mmap		= dt3155_mmap,
> +	.poll		= dt3155_poll,
> +	.open		= dt3155_open,
> +	.release	= dt3155_close
>  };
>  
>  
> diff --git a/drivers/staging/poch/poch.c b/drivers/staging/poch/poch.c
> index f940a34..bf46e29 100644
> --- a/drivers/staging/poch/poch.c
> +++ b/drivers/staging/poch/poch.c
> @@ -16,6 +16,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/poll.h>
>  #include <linux/idr.h>
> +#include <linux/smp_lock.h>
>  #include <linux/interrupt.h>
>  #include <linux/init.h>
>  #include <linux/ioctl.h>
> @@ -911,7 +912,7 @@ static unsigned int poch_poll(struct file *filp, poll_table *pt)
>  	return ret;
>  }
>  
> -static int poch_ioctl(struct inode *inode, struct file *filp,
> +static int poch_ioctl(struct file *filp,
>  		      unsigned int cmd, unsigned long arg)
>  {
>  	struct channel_info *channel = filp->private_data;
> @@ -1033,11 +1034,23 @@ static int poch_ioctl(struct inode *inode, struct file *filp,
>  	return 0;
>  }
>  
> +static long
> +poch_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	int ret;
> +
> +	lock_kernel();
> +	ret = poch_ioctl(file, cmd, arg);
> +	unlock_kernel();
> +
> +	return ret;
> +}
> +
>  static struct file_operations poch_fops = {
>  	.owner = THIS_MODULE,
>  	.open = poch_open,
>  	.release = poch_release,
> -	.ioctl = poch_ioctl,
> +	.poch_ioctl = poch_unlocked_ioctl,
>  	.poll = poch_poll,
>  	.mmap = poch_mmap
>  };
> diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
> index 1ab9a98..bc16fc0 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/syscalls.h>
> +#include <linux/smp_lock.h>
>  #include <linux/types.h>
>  
>  #include <asm/io.h>
> @@ -130,8 +131,7 @@ static int vme_user_release(struct inode *, struct file *);
>  static ssize_t vme_user_read(struct file *, char *, size_t, loff_t *);
>  static ssize_t vme_user_write(struct file *, const char *, size_t, loff_t *);
>  static loff_t vme_user_llseek(struct file *, loff_t, int);
> -static int vme_user_ioctl(struct inode *, struct file *, unsigned int,
> -	unsigned long);
> +static long vme_user_unlocked_ioctl(struct file *, unsigned int, unsigned long);
>  
>  static int __init vme_user_probe(struct device *, int, int);
>  static int __exit vme_user_remove(struct device *, int, int);
> @@ -142,7 +142,7 @@ static struct file_operations vme_user_fops = {
>          .read = vme_user_read,
>          .write = vme_user_write,
>          .llseek = vme_user_llseek,
> -        .ioctl = vme_user_ioctl,
> +        .unlocked_ioctl = vme_user_unlocked_ioctl,
>  };
>  
>  
> @@ -555,6 +555,18 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
>  	return -EINVAL;
>  }
>  
> +static long
> +vme_user_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	int ret;
> +
> +	lock_kernel();
> +	ret = vme_user_ioctl(file->f_path.dentry->d_inode, file, cmd, arg);
> +	unlock_kernel();
> +
> +	return ret;
> +}
> +
>  
>  /*
>   * Unallocate a previously allocated buffer
> -- 
> 1.7.0.4
> 

--
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/