[PATCH] USB: sisusbvga: Remove the BKL from open

[PATCH] USB: sisusbvga: Remove the BKL from open

[Alessio Igor Bogani]

BKL is not needed here because necessary locking is already provided
by mutex sisusb->lock.

Signed-off-by: Alessio Igor Bogani <abogani@texware.it>
---
 drivers/usb/misc/sisusbvga/sisusb.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c
index 63a6070..30d9303 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -47,7 +47,6 @@
 #include <linux/spinlock.h>
 #include <linux/kref.h>
 #include <linux/usb.h>
-#include <linux/smp_lock.h>
 #include <linux/vmalloc.h>
 
 #include "sisusb.h"
@@ -2416,14 +2415,11 @@ sisusb_open(struct inode *inode, struct file *file)
 	struct usb_interface *interface;
 	int subminor = iminor(inode);
 
-	lock_kernel();
 	if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
-		unlock_kernel();
 		return -ENODEV;
 	}
 
 	if (!(sisusb = usb_get_intfdata(interface))) {
-		unlock_kernel();
 		return -ENODEV;
 	}
 
@@ -2431,13 +2427,11 @@ sisusb_open(struct inode *inode, struct file *file)
 
 	if (!sisusb->present || !sisusb->ready) {
 		mutex_unlock(&sisusb->lock);
-		unlock_kernel();
 		return -ENODEV;
 	}
 
 	if (sisusb->isopen) {
 		mutex_unlock(&sisusb->lock);
-		unlock_kernel();
 		return -EBUSY;
 	}
 
@@ -2446,13 +2440,11 @@ sisusb_open(struct inode *inode, struct file *file)
 			if (sisusb_init_gfxdevice(sisusb, 0)) {
 				mutex_unlock(&sisusb->lock);
 				dev_err(&sisusb->sisusb_dev->dev, "Failed to initialize device\n");
-				unlock_kernel();
 				return -EIO;
 			}
 		} else {
 			mutex_unlock(&sisusb->lock);
 			dev_err(&sisusb->sisusb_dev->dev, "Device not attached to USB 2.0 hub\n");
-			unlock_kernel();
 			return -EIO;
 		}
 	}
@@ -2465,7 +2457,6 @@ sisusb_open(struct inode *inode, struct file *file)
 	file->private_data = sisusb;
 
 	mutex_unlock(&sisusb->lock);
-	unlock_kernel();
 
 	return 0;
 }
-- 
1.6.3.3

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Oliver Neukum]

Am Sonntag, 25. April 2010 12:37:10 schrieb Alessio Igor Bogani:
> BKL is not needed here because necessary locking is already provided
> by mutex sisusb->lock.

Have you checked the fb layer doesn't need it?

	Regards
		Oliver

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Alessio Igor Bogani]

Dear Mr. Neukum,

2010/4/26 Oliver Neukum <oneukum@suse.de>:
> Am Sonntag, 25. April 2010 12:37:10 schrieb Alessio Igor Bogani:
>> BKL is not needed here because necessary locking is already provided
>> by mutex sisusb->lock.
>
> Have you checked the fb layer doesn't need it?

The _open and _release functions are already serialized with mutex in
fb layer. So that mutex could be removed but in my opinion this job
should be done in a separate patch (like I have done some time ago for
nvidiafb driver). Now I would want suggest to remove BKL here only.

Ciao,
Alessio

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Arnd Bergmann]

On Monday 26 April 2010, Alessio Igor Bogani wrote:
> 2010/4/26 Oliver Neukum <oneukum@suse.de>:
> > Am Sonntag, 25. April 2010 12:37:10 schrieb Alessio Igor Bogani:
> >> BKL is not needed here because necessary locking is already provided
> >> by mutex sisusb->lock.
> >
> > Have you checked the fb layer doesn't need it?
> 
> The _open and _release functions are already serialized with mutex in
> fb layer. So that mutex could be removed but in my opinion this job
> should be done in a separate patch (like I have done some time ago for
> nvidiafb driver). Now I would want suggest to remove BKL here only.

What about the BKL in the sisusb_ioctl()? It seems to follow the
same logic, so I would think that you can simply remove the BKL
from the module entirely, rather than only half of it.

	Arnd

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Alessio Igor Bogani]

Dear Mr. Bergmann,

2010/4/26 Arnd Bergmann <arnd@arndb.de>:
>> >> BKL is not needed here because necessary locking is already provided
>> >> by mutex sisusb->lock.
[...]
> What about the BKL in the sisusb_ioctl()? It seems to follow the
> same logic, so I would think that you can simply remove the BKL
> from the module entirely, rather than only half of it.

It is what effectively I'm trying to do!

The first part of this work is already present in USB subsystem
maintainer tree (aka greg) at:
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/usb/usb-sisusbvga-remove-the-bkl-from-ioctl.patch

Ciao,
Alessio

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Arnd Bergmann]

On Monday 26 April 2010, Alessio Igor Bogani wrote:
> It is what effectively I'm trying to do!
> 
> The first part of this work is already present in USB subsystem
> maintainer tree (aka greg) at:
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/usb/usb-sisusbvga-remove-the-bkl-from-ioctl.patch
> 

Ok, looks good then. I just wouldn't bother splitting the work into two patches if you
want to do this for more drivers.

	Arnd

Re: [PATCH] USB: sisusbvga: Remove the BKL from open

[Alessio Igor Bogani]

Dear Mr. Bergmann,

2010/4/26 Arnd Bergmann <arnd@arndb.de>:
> On Monday 26 April 2010, Alessio Igor Bogani wrote:
>> It is what effectively I'm trying to do!
>>
>> The first part of this work is already present in USB subsystem
>> maintainer tree (aka greg) at:
>> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/usb/usb-sisusbvga-remove-the-bkl-from-ioctl.patch
>>
>
> Ok, looks good then. I just wouldn't bother splitting the work into two patches if you
> want to do this for more drivers.

You are right. Simply I made the above-mentioned patch when BKL
pushdown not yet reach _open function. :-)

Ciao,
Alessio