Re: Bug creating USB endpoints in 2.6.20.x (kernel bug 8198)
Tejun Heo <[email protected]>
| Newsgroups | gmane.linux.usb.devel |
|---|---|
| Message-ID | <[email protected]> |
Hello, Alan, Chris. Alan Stern wrote: > On Sat, 5 May 2007, Chris Rankin wrote: > >> --- Alan Stern <[email protected]> wrote: >>> However it is usually the same endpoint number and the same device (even >>> if the device number changes)? In that case we can tell the code where to >>> look for errors, because we will always know when that particular endpoint >>> file is created and when it is destroyed. >> I think it's safe to say "yes, it's always ep84" here, provided I am trying to provoke the bug >> with winecfg. I have obviously seen this bug in a few other scenarios as well over the months, >> e.g. from the report in bugzilla: > > Tejun, would this help? We could create and export a pointer to kobject > from the sysfs core, say: > > struct kobject *testptr; > EXPORT_SYMBOL(testptr); > > Then I could add something to the USB code which would set testptr to > point at the kobject embedded in the "ep84" device when it was created, > and would clear testptr when the device was released. This way you could > write a debugging patch for sysfs to monitor the sd associated with > testptr for nefarious changes. Yeap, good idea. I wrote something in sysfs proper. The patch creates a r/w module parameter /sys/module/dir/parameters/sysfs_trace_path to which you can echo the path of sysfs node to trace. For example, if you wanna trace tx_queue_len of dummy0, # echo -n "/sys/devices/virtual/net/dummy0/tx_queue_len" > /sys/module/dir/parameters/sysfs_trace_path Then, you'll get trace of creation/release, get/put and open/close which should tell us what's going on with the specific sysfs_dirent. Chris, can you apply this patch over 2.6.21.1 and echo /sys/class/usb_endpoint/usbdev1.3_ep84 (or whatever the problemetic node appears as) to it, trigger the bug and report what the kernel says? -- tejun ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
sysfs-debug.patch
(text/x-patch, 8 KB)
---
fs/sysfs/dir.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-----
fs/sysfs/file.c | 32 ++++++++++++++++++++++++--
fs/sysfs/symlink.c | 7 ++++-
fs/sysfs/sysfs.h | 25 +++++++++++++++++++-
4 files changed, 116 insertions(+), 12 deletions(-)
Index: tree0/fs/sysfs/dir.c
===================================================================
--- tree0.orig/fs/sysfs/dir.c
+++ tree0/fs/sysfs/dir.c
@@ -12,6 +12,9 @@
#include <asm/semaphore.h>
#include "sysfs.h"
+static char sysfs_trace_path[PATH_MAX];
+module_param_string(sysfs_trace_path, sysfs_trace_path, PATH_MAX, 0644);
+
DECLARE_RWSEM(sysfs_rename_sem);
static void sysfs_d_iput(struct dentry * dentry, struct inode * inode)
@@ -91,9 +94,53 @@ int sysfs_dirent_exist(struct sysfs_dire
return 0;
}
+void sysfs_trace_create(struct dentry *parent, const char *last_component,
+ struct sysfs_dirent *sd)
+{
+ char *buf, *path, *c;
+ int len;
+
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf) {
+ printk("sysfs_trace: OOPS no memory\n");
+ return;
+ }
+
+ path = d_path(parent, sysfs_mount, buf, PATH_MAX);
+ if (IS_ERR(path)) {
+ printk("sysfs_trace: OOPS d_path error %d\n",
+ (int)PTR_ERR(path));
+ goto out_free;
+ }
+
+ c = sysfs_trace_path;
+ if (strncmp(c, "/sys/", 5) == 0)
+ c += 4;
+
+ len = strlen(path);
+ if (strncmp(c, path, len))
+ goto out_free;
+
+ /* only the last component should be remaining */
+ c += len;
+ if (*c++ != '/')
+ goto out_free;
+
+ if (strcmp(c, last_component))
+ goto out_free;
+
+ printk("sysfs_trace: CREATE %s pid=%u comm=%s",
+ sysfs_trace_path, current->pid, current->comm);
+ dump_stack();
+ sd->s_trace = 1;
+
+ out_free:
+ kfree(buf);
+}
static struct sysfs_dirent *
-__sysfs_make_dirent(struct dentry *dentry, void *element, mode_t mode, int type)
+__sysfs_make_dirent(struct dentry *dentry, void *element, mode_t mode, int type,
+ struct sysfs_dirent **psd)
{
struct sysfs_dirent * sd;
@@ -104,6 +151,8 @@ __sysfs_make_dirent(struct dentry *dentr
sd->s_mode = mode;
sd->s_type = type;
sd->s_dentry = dentry;
+ if (psd)
+ *psd = sd;
if (dentry) {
dentry->d_fsdata = sysfs_get(sd);
dentry->d_op = &sysfs_dentry_ops;
@@ -114,11 +163,12 @@ out:
}
int sysfs_make_dirent(struct sysfs_dirent * parent_sd, struct dentry * dentry,
- void * element, umode_t mode, int type)
+ void * element, umode_t mode, int type,
+ struct sysfs_dirent **psd)
{
struct sysfs_dirent *sd;
- sd = __sysfs_make_dirent(dentry, element, mode, type);
+ sd = __sysfs_make_dirent(dentry, element, mode, type, psd);
__sysfs_list_dirent(parent_sd, sd);
return sd ? 0 : -ENOMEM;
@@ -152,6 +202,7 @@ static int create_dir(struct kobject * k
{
int error;
umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
+ struct sysfs_dirent *sd;
mutex_lock(&p->d_inode->i_mutex);
*d = lookup_one_len(n, p, strlen(n));
@@ -160,8 +211,10 @@ static int create_dir(struct kobject * k
error = -EEXIST;
else
error = sysfs_make_dirent(p->d_fsdata, *d, k, mode,
- SYSFS_DIR);
+ SYSFS_DIR, &sd);
+
if (!error) {
+ sysfs_trace_create(p, n, sd);
error = sysfs_create(*d, mode, init_dir);
if (!error) {
inc_nlink(p->d_inode);
@@ -170,7 +223,6 @@ static int create_dir(struct kobject * k
}
}
if (error && (error != -EEXIST)) {
- struct sysfs_dirent *sd = (*d)->d_fsdata;
if (sd) {
list_del_init(&sd->s_sibling);
sysfs_put(sd);
@@ -651,7 +703,7 @@ struct dentry *sysfs_create_shadow_dir(s
if (!shadow)
goto nomem;
- sd = __sysfs_make_dirent(shadow, kobj, inode->i_mode, SYSFS_DIR);
+ sd = __sysfs_make_dirent(shadow, kobj, inode->i_mode, SYSFS_DIR, NULL);
if (!sd)
goto nomem;
Index: tree0/fs/sysfs/file.c
===================================================================
--- tree0.orig/fs/sysfs/file.c
+++ tree0/fs/sysfs/file.c
@@ -354,6 +354,18 @@ static int sysfs_open_file(struct inode
file->private_data = buffer;
} else
error = -ENOMEM;
+
+ {
+ struct sysfs_dirent *sd = file->f_path.dentry->d_fsdata;
+
+ if (sd->s_trace) {
+ printk("sysfs_trace: OPEN s_count=%d pid=%u comm=%s",
+ atomic_read(&sd->s_count), current->pid,
+ current->comm);
+ dump_stack();
+ }
+ }
+
goto Done;
Einval:
@@ -375,6 +387,17 @@ static int sysfs_release(struct inode *
struct module * owner = attr->owner;
struct sysfs_buffer * buffer = filp->private_data;
+ {
+ struct sysfs_dirent *sd = filp->f_path.dentry->d_fsdata;
+
+ if (sd->s_trace) {
+ printk("sysfs_trace: CLOSE s_count=%d pid=%u comm=%s",
+ atomic_read(&sd->s_count), current->pid,
+ current->comm);
+ dump_stack();
+ }
+ }
+
if (buffer)
remove_from_collection(buffer, inode);
kobject_put(kobj);
@@ -386,6 +409,7 @@ static int sysfs_release(struct inode *
free_page((unsigned long)buffer->page);
kfree(buffer);
}
+
return 0;
}
@@ -477,9 +501,13 @@ int sysfs_add_file(struct dentry * dir,
int error = -EEXIST;
mutex_lock(&dir->d_inode->i_mutex);
- if (!sysfs_dirent_exist(parent_sd, attr->name))
+ if (!sysfs_dirent_exist(parent_sd, attr->name)) {
+ struct sysfs_dirent *sd;
error = sysfs_make_dirent(parent_sd, NULL, (void *)attr,
- mode, type);
+ mode, type, &sd);
+ if (!error)
+ sysfs_trace_create(dir, attr->name, sd);
+ }
mutex_unlock(&dir->d_inode->i_mutex);
return error;
Index: tree0/fs/sysfs/symlink.c
===================================================================
--- tree0.orig/fs/sysfs/symlink.c
+++ tree0/fs/sysfs/symlink.c
@@ -49,6 +49,7 @@ static int sysfs_add_link(struct dentry
{
struct sysfs_dirent * parent_sd = parent->d_fsdata;
struct sysfs_symlink * sl;
+ struct sysfs_dirent *sd;
int error = 0;
error = -ENOMEM;
@@ -64,9 +65,11 @@ static int sysfs_add_link(struct dentry
sl->target_kobj = kobject_get(target);
error = sysfs_make_dirent(parent_sd, NULL, sl, S_IFLNK|S_IRWXUGO,
- SYSFS_KOBJ_LINK);
- if (!error)
+ SYSFS_KOBJ_LINK, &sd);
+ if (!error) {
+ sysfs_trace_create(parent, name, sd);
return 0;
+ }
kobject_put(target);
kfree(sl->link_name);
Index: tree0/fs/sysfs/sysfs.h
===================================================================
--- tree0.orig/fs/sysfs/sysfs.h
+++ tree0/fs/sysfs/sysfs.h
@@ -8,6 +8,7 @@ struct sysfs_dirent {
struct dentry * s_dentry;
struct iattr * s_iattr;
atomic_t s_event;
+ int s_trace:1;
};
extern struct vfsmount * sysfs_mount;
@@ -19,7 +20,10 @@ extern int sysfs_create(struct dentry *,
extern int sysfs_dirent_exist(struct sysfs_dirent *, const unsigned char *);
extern int sysfs_make_dirent(struct sysfs_dirent *, struct dentry *, void *,
- umode_t, int);
+ umode_t, int, struct sysfs_dirent **);
+extern void sysfs_trace_create(struct dentry *dentry,
+ const char *last_component,
+ struct sysfs_dirent *sd);
extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
extern int sysfs_hash_and_remove(struct dentry * dir, const char * name);
@@ -97,8 +101,15 @@ static inline struct kobject *sysfs_get_
return kobj;
}
+#include <linux/sched.h>
static inline void release_sysfs_dirent(struct sysfs_dirent * sd)
{
+ if (sd->s_trace) {
+ printk("sysfs_trace: RELEASE pid=%u comm=%s",
+ current->pid, current->comm);
+ dump_stack();
+ }
+
if (sd->s_type & SYSFS_KOBJ_LINK) {
struct sysfs_symlink * sl = sd->s_element;
kfree(sl->link_name);
@@ -114,14 +125,24 @@ static inline struct sysfs_dirent * sysf
if (sd) {
WARN_ON(!atomic_read(&sd->s_count));
atomic_inc(&sd->s_count);
+ if (sd->s_trace) {
+ printk("sysfs_trace: GET s_count=%d pid=%u comm=%s",
+ atomic_read(&sd->s_count), current->pid, current->comm);
+ dump_stack();
+ }
}
return sd;
}
static inline void sysfs_put(struct sysfs_dirent * sd)
{
- if (atomic_dec_and_test(&sd->s_count))
+ if (atomic_dec_and_test(&sd->s_count)) {
release_sysfs_dirent(sd);
+ } else if (sd->s_trace) {
+ printk("sysfs_trace: PUT s_count=%d pid=%u comm=%s",
+ atomic_read(&sd->s_count), current->pid, current->comm);
+ dump_stack();
+ }
}
static inline int sysfs_is_shadowed_inode(struct inode *inode)