2010-04-25 10:37:25

by Alessio Igor Bogani

[permalink] [raw]
Subject: [PATCH] USB: sisusbvga: Remove the BKL from open

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

Signed-off-by: Alessio Igor Bogani <[email protected]>
---
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


2010-04-26 12:06:54

by Oliver Neukum

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

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

2010-04-26 12:30:53

by Alessio Igor Bogani

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

Dear Mr. Neukum,

2010/4/26 Oliver Neukum <[email protected]>:
> 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

2010-04-26 13:16:22

by Arnd Bergmann

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

On Monday 26 April 2010, Alessio Igor Bogani wrote:
> 2010/4/26 Oliver Neukum <[email protected]>:
> > 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

2010-04-26 13:30:38

by Alessio Igor Bogani

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

Dear Mr. Bergmann,

2010/4/26 Arnd Bergmann <[email protected]>:
>> >> 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

2010-04-26 13:51:05

by Arnd Bergmann

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

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

2010-04-26 14:09:08

by Alessio Igor Bogani

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

Dear Mr. Bergmann,

2010/4/26 Arnd Bergmann <[email protected]>:
> 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