diff mbox

[Bug,#15589] 2.6.34-rc1: Badness at fs/proc/generic.c:316

Message ID 1271765898.4324.2.camel@concordia (mailing list archive)
State Changes Requested
Headers show

Commit Message

Michael Ellerman April 20, 2010, 12:18 p.m. UTC 
On Mon, 2010-04-19 at 23:45 -0700, Christian Kujau wrote:
> On Tue, 20 Apr 2010 at 05:19, Rafael J. Wysocki wrote:
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=15589
> > Subject		: 2.6.34-rc1: Badness at fs/proc/generic.c:316
> > Submitter	: Christian Kujau <lists@nerdbynature.de>
> > Date		: 2010-03-13 23:53 (38 days old)
> > Message-ID	: <alpine.DEB.2.01.1003131544340.5493@bogon.housecafe.de>
> > References	: http://marc.info/?l=linux-kernel&m=126852442903680&w=2
> 
> Yes, unless something in this area has changed from -rc4 to -rc5, this is 
> still printed during boot:
> 
> 
> device-tree: Duplicate name in /cpus/PowerPC,G4@0, renamed to "l2-cache#1"
> name 'pulses/rev'
> ------------[ cut here ]------------

Don't cut here, sigh.

> Badness at fs/proc/generic.c:317

Try this 100% unbuilt, 100% untested patch.

cheers

Comments

Michael Ellerman April 21, 2010, 12:21 a.m. UTC | #1 
On Wed, 2010-04-21 at 18:55 +0300, Alexey Dobriyan wrote:
> On Tue, Apr 20, 2010 at 10:18:18PM +1000, Michael Ellerman wrote:
> > On Mon, 2010-04-19 at 23:45 -0700, Christian Kujau wrote:
> > --- a/fs/proc/proc_devtree.c
> > +++ b/fs/proc/proc_devtree.c
> > @@ -175,6 +175,24 @@ retry:
> >         return fixed_name;
> >  }
> >  
> > +static const char *unslash_name(const char *name)
> > +{
> > +       char *p, *fixed_name;
> > +
> > +       fixed_name = kstrdup(name);
> > +       if (!fixed_name) {
> > +               printk(KERN_ERR "device-tree: Out of memory trying to unslash "
> > +                               "name \"%s\"\n", name);
> > +               return name;
> > +       }
> > +
> > +       p = fixed_name;
> > +       while ((p = strstr(p, "/")))
> > +               *p++ = '_';
> 
> This is wasteful. :-)

Whatever, patches welcome :)

> Also, I hope we won't spit message every time allocation fail.

We do. Your system is mostly hosed anyway, but feel free to rate limit
it or something.

The error handling in there is a bit dubious, if the alloc fails we just
return the old name, which we know is bogus. It should probably return
NULL and the calling code can check - same for fixup_name().

cheers
Rafael J. Wysocki April 21, 2010, 4:57 a.m. UTC | #2 
On Wednesday 21 April 2010, Michael Ellerman wrote:
> On Wed, 2010-04-21 at 18:55 +0300, Alexey Dobriyan wrote:
> > On Tue, Apr 20, 2010 at 10:18:18PM +1000, Michael Ellerman wrote:
> > > On Mon, 2010-04-19 at 23:45 -0700, Christian Kujau wrote:
> > > --- a/fs/proc/proc_devtree.c
> > > +++ b/fs/proc/proc_devtree.c
> > > @@ -175,6 +175,24 @@ retry:
> > >         return fixed_name;
> > >  }
> > >  
> > > +static const char *unslash_name(const char *name)
> > > +{
> > > +       char *p, *fixed_name;
> > > +
> > > +       fixed_name = kstrdup(name);
> > > +       if (!fixed_name) {
> > > +               printk(KERN_ERR "device-tree: Out of memory trying to unslash "
> > > +                               "name \"%s\"\n", name);
> > > +               return name;
> > > +       }
> > > +
> > > +       p = fixed_name;
> > > +       while ((p = strstr(p, "/")))
> > > +               *p++ = '_';
> > 
> > This is wasteful. :-)
> 
> Whatever, patches welcome :)
> 
> > Also, I hope we won't spit message every time allocation fail.
> 
> We do. Your system is mostly hosed anyway, but feel free to rate limit
> it or something.

OK

Is anyone going to post a clean patch for that with a sign-off?

Rafael
Alexey Dobriyan April 21, 2010, 3:55 p.m. UTC | #3 
On Tue, Apr 20, 2010 at 10:18:18PM +1000, Michael Ellerman wrote:
> On Mon, 2010-04-19 at 23:45 -0700, Christian Kujau wrote:
> --- a/fs/proc/proc_devtree.c
> +++ b/fs/proc/proc_devtree.c
> @@ -175,6 +175,24 @@ retry:
>         return fixed_name;
>  }
>  
> +static const char *unslash_name(const char *name)
> +{
> +       char *p, *fixed_name;
> +
> +       fixed_name = kstrdup(name);
> +       if (!fixed_name) {
> +               printk(KERN_ERR "device-tree: Out of memory trying to unslash "
> +                               "name \"%s\"\n", name);
> +               return name;
> +       }
> +
> +       p = fixed_name;
> +       while ((p = strstr(p, "/")))
> +               *p++ = '_';

This is wasteful. :-)
Also, I hope we won't spit message every time allocation fail.
diff mbox

Patch

diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c
index f8650dc..9502b48 100644
--- a/fs/proc/proc_devtree.c
+++ b/fs/proc/proc_devtree.c
@@ -175,6 +175,24 @@  retry:
        return fixed_name;
 }
 
+static const char *unslash_name(const char *name)
+{
+       char *p, *fixed_name;
+
+       fixed_name = kstrdup(name);
+       if (!fixed_name) {
+               printk(KERN_ERR "device-tree: Out of memory trying to unslash "
+                               "name \"%s\"\n", name);
+               return name;
+       }
+
+       p = fixed_name;
+       while ((p = strstr(p, "/")))
+               *p++ = '_';
+
+       return fixed_name;
+}
+
 /*
  * Process a node, adding entries for its children and its properties.
  */
@@ -211,6 +229,9 @@  void proc_device_tree_add_node(struct device_node *np,
                if (duplicate_name(de, p))
                        p = fixup_name(np, de, p);
 
+               if (strstr(p, "/"))
+                       p = unslash_name(p);
+
                ent = __proc_device_tree_add_prop(de, pp, p);
                if (ent == NULL)
                        break;