changed ioctls to unlocked
PATCH 6/13 changed ioctls to unlocked
From: Stoyan Gaydarov <[email protected]>
--- drivers/scsi/osst.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
drivers/scsi/osst.c +8 -4
@@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)/* The ioctl command */ -static int osst_ioctl (struct inode * inode,struct file * file, -unsigned int cmd_in,unsigned long arg) +static long osst_ioctl (struct file * file,unsigned int cmd_in, +unsigned long arg) { +lock_kernel ();int i, cmd_nr, cmd_type, blk, retval =0 ;struct st_modedef * STm;struct st_partstat * STps;
@@ -4867,8 +4868,10 @@ static int osst_ioctl(struct inode * inode,struct file * file,char * name =tape_name (STp);void __user * p = (void __user *)arg; -if (mutex_lock_interruptible (&STp->lock)) +if (mutex_lock_interruptible (&STp->lock)) { +unlock_kernel ();return -ERESTARTSYS ; + }#if DEBUG if (debugging && !STp->in_use) {
@@ -5187,6 +5190,7 @@ out:mutex_unlock (&STp->lock); +unlock_kernel ();return retval; }
@@ -5542,7 +5546,7 @@ static const struct file_operations osst_fops = { .owner = THIS_MODULE, .read = osst_read, .write = osst_write, - .ioctl = osst_ioctl, + .unlocked_ioctl = osst_ioctl,#ifdef CONFIG_COMPAT .compat_ioctl = osst_compat_ioctl,#endif
Signature · 1 line
1.6.2
James Bottomley
maintainerQuoted 12 lines · On Tue, 2009-03-24 at 16:12 -0500, [email protected] wrote:
From: Stoyan Gaydarov <[email protected]>
Signed-off-by Stoyan Gaydarov <[email protected]>--- drivers/scsi/osst.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)drivers/scsi/osst.c +0
-0
drivers/scsi/osst.c static int os_scsi_tape_close(struct inode * inode, struct file * filp) · line 4862
Quoted 9 lines
drivers/scsi/osst.c +3
-2@@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)/* The ioctl command */ -static int osst_ioctl (struct inode * inode,struct file * file, -unsigned int cmd_in,unsigned long arg) +static long osst_ioctl (struct file * file,unsigned int cmd_in, +unsigned long arg) { +lock_kernel ();
What necessitates the kernel locking? When st was audited, it didn't need it; since osst is in many ways a copy of st, I'm surprised it does.
James
Quoted 26 lines · James Bottomley
Quoted 12 lines · On Tue, 2009-03-24 at 16:12 -0500, [email protected] wrote:
From: Stoyan Gaydarov <[email protected]>
Signed-off-by Stoyan Gaydarov <[email protected]>---
drivers/scsi/osst.c | 12 ++++++++----1 files changed, 8 insertions(+), 4 deletions(-)drivers/scsi/osst.c +0
-0drivers/scsi/osst.c static int os_scsi_tape_close(struct inode * inode, struct file * filp) · line 4862
Quoted 9 lines
drivers/scsi/osst.c +3
-2@@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp) /* The ioctl command */ -static int osst_ioctl (struct inode * inode,struct file * file, - unsigned int cmd_in, unsigned long arg) +static long osst_ioctl (struct file * file,unsigned int cmd_in, + unsigned long arg) { + lock_kernel();What necessitates the kernel locking? When st was audited, it didn't need it; since osst is in many ways a copy of st, I'm surprised it does.
Quoted 1 line
James
The lock was there before, because it is used before calling osst_ioctl, my patches don't change any functionality. It is meant to show the maintainers of the code that the lock is there and to allow them make a decision as to how best to handle the code without the kernel locking. If it is not needed then the locks can be removed and the function call can remain under the unlocked_ioctl.
-Stoyan
Quoted 3 lines
Signature · 5 lines
-Stoyan -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
drivers/scsi/osst.c static int os_scsi_tape_close(struct inode * inode, struct file * filp) · line 4863
Quoted 5 lines · On Tue, Mar 24, 2009 at 04:12:41PM -0500, [email protected] wrote:
drivers/scsi/osst.c +3
-0+static long osst_ioctl (struct file * file,unsigned int cmd_in, +unsigned long arg) { +lock_kernel ();int i, cmd_nr, cmd_type, blk, retval =0 ;
Did you try compiling this? If so, what compiler did you use? You *should* have got an error about mixing code and declarations.
Signature · 4 lines
Matthew Wilcox Intel Open Source Technology Centre "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."