David Brownell wrote:
> On Tuesday 29 August 2006 9:35 am, Sam Bishop wrote:
>> On Tue, Aug 29, 2006 at 08:30:45AM -0700, David Brownell wrote:
>>> On Monday 28 August 2006 1:27 pm, Sam Bishop wrote:
>>>> + /* To see what's changed, compare the file's previous and current
>>>> + contents or scan the filesystem. (Scanning is more precise.) */
>>> Rather than "more precise", "less error prone" ... there are nasty error
>>> paths in the "compare" case. Better yet, don't mention that.
>> Sure. I also removed the bit where I call HAL a program...
>
> Erm, what I meant was removing mention of the error prone "compare" case.
> If you're going to mention it, highlight that it's got internal races;
> otherwise, don't mention it.
>
Something I'm missing here but polling on /proc/bus/usb/devices don't
work for me...I attached the prog I used to test that. You can simply
do in order to test it:
$ gcc poll.c
$ ./a.out /prob/bus/usb/devices > /dev/null
If no event occurs within 5 seconds, it will timeout.
I took a look at drivers/usb/core/devices.c and it seems to be broken.
This patch helps to make things work...
Thanks
Franck
-- >8 --
Subject: [PATCH] usbfs: remove BKL usage in devices.c
Now the usb_device_status structure is allocated in usbdev_open().
So we don't need to protect it anymore.
This patch also fixes a bug in usb_device_poll() at the same time.
Previous code always raised POLLIN bit although no event happened
on the bus.
Signed-off-by: Franck Bui-Huu <[EMAIL PROTECTED]>
---
drivers/usb/core/devices.c | 55 +++++++++++++++++---------------------------
1 files changed, 21 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index c0f3734..d385b13 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -123,6 +123,7 @@ static unsigned int conndiscevcnt = 0;
/* this struct stores the poll state for <mountpoint>/devices pollers */
struct usb_device_status {
unsigned int lastev;
+ struct mutex lock;
};
struct class_info {
@@ -590,50 +591,38 @@ static ssize_t usb_device_read(struct fi
return total_written;
}
-/* Kernel lock for "lastev" protection */
static unsigned int usb_device_poll(struct file *file, struct
poll_table_struct *wait)
{
- struct usb_device_status *st = (struct usb_device_status
*)file->private_data;
+ struct usb_device_status *st;
unsigned int mask = 0;
- lock_kernel();
- if (!st) {
- st = kmalloc(sizeof(struct usb_device_status), GFP_KERNEL);
- if (!st) {
- unlock_kernel();
- return POLLIN;
- }
-
- /* we may have dropped BKL - need to check for having lost the
race */
- if (file->private_data) {
- kfree(st);
- st = file->private_data;
- goto lost_race;
- }
-
- /*
- * need to prevent the module from being unloaded, since
- * proc_unregister does not call the release method and
- * we would have a memory leak
- */
- st->lastev = conndiscevcnt;
- file->private_data = st;
- mask = POLLIN;
- }
-lost_race:
if (file->f_mode & FMODE_READ)
poll_wait(file, &deviceconndiscwq, wait);
+
+ st = (struct usb_device_status *)file->private_data;
+
+ mutex_lock(&st->lock);
if (st->lastev != conndiscevcnt)
mask |= POLLIN;
st->lastev = conndiscevcnt;
- unlock_kernel();
+ mutex_unlock(&st->lock);
+
return mask;
}
static int usb_device_open(struct inode *inode, struct file *file)
{
- file->private_data = NULL;
- return 0;
+ struct usb_device_status *st;
+ int rv = -ENOMEM;
+
+ st = kmalloc(sizeof(struct usb_device_status), GFP_KERNEL);
+ if (st) {
+ mutex_init(&st->lock);
+ st->lastev = conndiscevcnt;
+ file->private_data = st;
+ rv = 0;
+ }
+ return rv;
}
static int usb_device_release(struct inode *inode, struct file *file)
@@ -647,8 +636,7 @@ static loff_t usb_device_lseek(struct fi
{
loff_t ret;
- lock_kernel();
-
+ mutex_lock(&file->f_dentry->d_inode->i_mutex);
switch (orig) {
case 0:
file->f_pos = offset;
@@ -662,8 +650,7 @@ static loff_t usb_device_lseek(struct fi
default:
ret = -EINVAL;
}
-
- unlock_kernel();
+ mutex_unlock(&file->f_dentry->d_inode->i_mutex);
return ret;
}
--
1.4.2
#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/poll.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#define POLL_TIMEOUT_MS 5000
static void die(const char *err, ...)
{
va_list params;
va_start(params, err);
fputs("*** ", stderr);
vfprintf(stderr, err, params);
fputs("\n", stderr);
va_end(params);
exit(128);
}
static void dump_fd(int fd)
{
static char buf[4096];
ssize_t count;
int i;
while ((count = read(fd, buf, 4096)))
for (i = 0; i < count; i++)
putchar(buf[i]);
}
int main(int argc, char **argv)
{
struct pollfd ufds;
int fd;
int rv;
if (argc != 2)
die("you must specify one file to poll");
fd = open(argv[1], O_RDONLY);
if (fd < 1)
die("cannot open %s", argv[1]);
ufds.fd = fd;
ufds.events = POLLIN;
ufds.revents = 0;
rv = poll(&ufds, 1, POLL_TIMEOUT_MS);
if (rv < 0)
die("poll failed with %d", rv);
if (rv) {
fprintf(stderr, "poll returns %d, revents = %x\n", rv, ufds.revents);
dump_fd(fd);
} else
fprintf(stderr, "timeout !\n");
close(fd);
return 0;
}
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
