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

Reply via email to