2008-01-14 14:31:28

by Mathieu Segaud

Subject: [PATCH] Convert drivers/scsi/ch.c to use unlocked_ioctl

As of now, compat_ioctl already runs without the BKL, whereas ioctl runs
with the BKL. This patch first converts changer_fops to use a .unlocked_ioctl
member. It applies the same locking rationale than ch_ioctl_compat() uses
to ch_ioctl().

Signed-off-by: Mathieu Segaud <[email protected]>
---
drivers/scsi/ch.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 2311019..389777c 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -92,8 +92,7 @@ static int ch_probe(struct device *);
static int ch_remove(struct device *);
static int ch_open(struct inode * inode, struct file * filp);
static int ch_release(struct inode * inode, struct file * filp);
-static int ch_ioctl(struct inode * inode, struct file * filp,
- unsigned int cmd, unsigned long arg);
+static long ch_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
#ifdef CONFIG_COMPAT
static long ch_ioctl_compat(struct file * filp,
unsigned int cmd, unsigned long arg);
@@ -130,12 +129,12 @@ static struct scsi_driver ch_template =

static const struct file_operations changer_fops =
{
- .owner = THIS_MODULE,
- .open = ch_open,
- .release = ch_release,
- .ioctl = ch_ioctl,
+ .owner = THIS_MODULE,
+ .open = ch_open,
+ .release = ch_release,
+ .unlocked_ioctl = ch_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = ch_ioctl_compat,
+ .compat_ioctl = ch_ioctl_compat,
#endif
};

@@ -626,7 +625,7 @@ ch_checkrange(scsi_changer *ch, unsigned int type, unsigned int unit)
return 0;
}

-static int ch_ioctl(struct inode * inode, struct file * file,
+static long ch_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
scsi_changer *ch = file->private_data;
@@ -887,8 +886,7 @@ static long ch_ioctl_compat(struct file * file,
case CHIOINITELEM:
case CHIOSVOLTAG:
/* compatible */
- return ch_ioctl(NULL /* inode, unused */,
- file, cmd, arg);
+ return ch_ioctl(file, cmd, arg);
case CHIOGSTATUS32:
{
struct changer_element_status32 ces32;
--
1.5.3.8

2008-01-14 14:38:10

by Matthew Wilcox

Subject: Re: [PATCH] Convert drivers/scsi/ch.c to use unlocked_ioctl

On Mon, Jan 14, 2008 at 03:31:17PM +0100, Mathieu Segaud wrote:
> + .owner = THIS_MODULE,
> + .open = ch_open,
> + .release = ch_release,
> + .unlocked_ioctl = ch_ioctl,

Do you have a tab setting that's not 8? You seem to have used one tab
too many for the first three lines.

Please correct that one, and then add my:

Reviewed-by: Matthew Wilcox <[email protected]>

Thanks!

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."