Re: 2.6.22-rc6-mm1 -- BUG - EIP: [<c01a77a1>] sysfs_addrm_finish+0x1c2/0x226 SS:ESP 0068:c5ff9db8

From: Miles Lane
Date: Wed Jul 11 2007 - 18:39:59 EST

On 7/11/07, Tejun Heo <htejun@xxxxxxxxx> wrote:
Tejun Heo wrote:
> Miles Lane wrote:
>>> Thanks a lot. Just in case, if you remove the patch (patch -R -p1), the
>>> oops goes away, right?
>> I double-checked. I can boot fine after building without your patch.
>> Also, I reproduced the initial BUG I reported (triggered by
>> "modprobe -r ipw2200").
>
> This is creepy. I was able to reproduce the oops here with your
> configuration file and making buffers for kallsyms static solved the
> problem. It isn't stack overflow. At maximum those arrays added 254
> bytes to the stack and when the oops occurs stack area was left more
> than enough. I'll keep looking into why that happened but the attached
> patch should get us going on the original subject.

Alright, found out what was going on. KSYM_NAME_LEN doesn't include
space for the trailing '\0'. Gees, I've read enough assembly for the
month to find that out. Anyways, here's proper debug patch.

Thanks.

--
tejun

---
fs/sysfs/dir.c | 43 +++++++++++++++++++++++++++++++++++++++++++
fs/sysfs/sysfs.h | 16 ++--------------
include/linux/sysfs.h | 3 +++
net/core/net-sysfs.c | 6 +++++-
4 files changed, 53 insertions(+), 15 deletions(-)

Index: tree0/fs/sysfs/dir.c
===================================================================
--- tree0.orig/fs/sysfs/dir.c
+++ tree0/fs/sysfs/dir.c
@@ -11,15 +11,52 @@
#include <linux/namei.h>
#include <linux/idr.h>
#include <linux/completion.h>
+#include <linux/kallsyms.h>
#include <asm/semaphore.h>
#include "sysfs.h"

+struct kobject *sysfs_debug_me;
+
DEFINE_MUTEX(sysfs_mutex);
spinlock_t sysfs_assoc_lock = SPIN_LOCK_UNLOCKED;

static spinlock_t sysfs_ino_lock = SPIN_LOCK_UNLOCKED;
static DEFINE_IDA(sysfs_ino_ida);

+struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd)
+{
+ if (sd) {
+ if (sd->s_flags & SYSFS_FLAG_XXX) {
+ char c0[KSYM_NAME_LEN + 1], c1[KSYM_NAME_LEN + 1];
+
+ lookup_symbol_name((unsigned long)__builtin_return_address(0), c0);
+ lookup_symbol_name((unsigned long)__builtin_return_address(1), c1);
+
+ printk("sysfs_get(%s): cnt=%d++ called from %s:%s\n",
+ sd->s_name, atomic_read(&sd->s_count), c0, c1);
+ }
+ WARN_ON(!atomic_read(&sd->s_count));
+ atomic_inc(&sd->s_count);
+ }
+ return sd;
+}
+
+void sysfs_put(struct sysfs_dirent * sd)
+{
+ if (sd && sd->s_flags & SYSFS_FLAG_XXX) {
+ char c0[KSYM_NAME_LEN + 1], c1[KSYM_NAME_LEN + 1];
+
+ lookup_symbol_name((unsigned long)__builtin_return_address(0), c0);
+ lookup_symbol_name((unsigned long)__builtin_return_address(1), c1);
+
+ printk("sysfs_put(%s): cnt=%d-- called from %s:%s\n",
+ sd->s_name, atomic_read(&sd->s_count), c0, c1);
+ }
+
+ if (sd && atomic_dec_and_test(&sd->s_count))
+ release_sysfs_dirent(sd);
+}
+
/**
* sysfs_link_sibling - link sysfs_dirent into sibling list
* @sd: sysfs_dirent of interest
@@ -317,6 +354,10 @@ void release_sysfs_dirent(struct sysfs_d
* sd->s_parent won't change beneath us.
*/
parent_sd = sd->s_parent;
+ if (parent_sd->s_flags & SYSFS_FLAG_XXX)
+ printk("put from release(%s): cnt=%d-- (rel=%s)\n",
+ parent_sd->s_name, atomic_read(&parent_sd->s_count),
+ sd->s_name);

if (sysfs_type(sd) == SYSFS_KOBJ_LINK)
sysfs_put(sd->s_elem.symlink.target_sd);
@@ -695,6 +736,8 @@ static int create_dir(struct kobject *ko
if (!sd)
return -ENOMEM;
sd->s_elem.dir.kobj = kobj;
+ if (sysfs_debug_me && sysfs_debug_me == kobj)
+ sd->s_flags |= SYSFS_FLAG_XXX;

/* link in */
sysfs_addrm_start(&acxt, parent_sd);
Index: tree0/fs/sysfs/sysfs.h
===================================================================
--- tree0.orig/fs/sysfs/sysfs.h
+++ tree0/fs/sysfs/sysfs.h
@@ -108,20 +108,8 @@ static inline unsigned int sysfs_type(st
return sd->s_flags & SYSFS_TYPE_MASK;
}

-static inline struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd)
-{
- if (sd) {
- WARN_ON(!atomic_read(&sd->s_count));
- atomic_inc(&sd->s_count);
- }
- return sd;
-}
-
-static inline void sysfs_put(struct sysfs_dirent * sd)
-{
- if (sd && atomic_dec_and_test(&sd->s_count))
- release_sysfs_dirent(sd);
-}
+struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd);
+void sysfs_put(struct sysfs_dirent * sd);

static inline int sysfs_is_shadowed_inode(struct inode *inode)
{
Index: tree0/include/linux/sysfs.h
===================================================================
--- tree0.orig/include/linux/sysfs.h
+++ tree0/include/linux/sysfs.h
@@ -87,9 +87,12 @@ struct sysfs_ops {

#define SYSFS_FLAG_MASK ~SYSFS_TYPE_MASK
#define SYSFS_FLAG_REMOVED 0x0100
+#define SYSFS_FLAG_XXX 0x0200

#ifdef CONFIG_SYSFS

+extern struct kobject *sysfs_debug_me;
+
extern int sysfs_schedule_callback(struct kobject *kobj,
void (*func)(void *), void *data, struct module *owner);

Index: tree0/net/core/net-sysfs.c
===================================================================
--- tree0.orig/net/core/net-sysfs.c
+++ tree0/net/core/net-sysfs.c
@@ -472,6 +472,7 @@ int netdev_register_sysfs(struct net_dev
{
struct device *dev = &(net->dev);
struct attribute_group **groups = net->sysfs_groups;
+ int rc;

device_initialize(dev);
dev->class = &net_class;
@@ -489,7 +490,10 @@ int netdev_register_sysfs(struct net_dev
*groups++ = &wireless_group;
#endif

- return device_add(dev);
+ sysfs_debug_me = &dev->kobj;
+ rc = device_add(dev);
+ sysfs_debug_me = NULL;
+ return rc;
}

int netdev_sysfs_init(void)



Tejun,

Does this patch replace or add to the last patch you sent me?
I'll try applying them with --dry-run and assume that if #1 + #2
doesn't apply cleanly, then you only want me to test with this
latest patch.

I'll be able to test this later this evening. I've had house guests and
so haven't been available the last few days.

Miles
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/