Re: [PATCH] Fix Kconfig performance bug

From: David Gibson (david_at_gibson.dropbear.id.au)
Date: 10/31/05

  • Next message: Dmitry Torokhov: "[GIT PULL] Tiny input update"
    Date:	Mon, 31 Oct 2005 17:55:21 +1100
    To: Roman Zippel <zippel@linux-m68k.org>, kbuild-devel@lists.sourceforge.net, linuxppc64-dev@ozlabs.org, linux-kernel@vger.kernel.org
    
    

    On Fri, Oct 21, 2005 at 11:49:55AM +1000, David Gibson wrote:
    > On Fri, Oct 21, 2005 at 02:46:30AM +0200, Roman Zippel wrote:
    > > Hi,
    > >
    > > On Thu, 20 Oct 2005, David Gibson wrote:
    > >
    > > > When doing its recursive dependency check, scripts/kconfig/conf uses
    > > > the flag SYMBOL_CHECK_DONE to avoid rechecking a symbol it has already
    > > > checked. However, that flag is only set at the top level, so if a
    > > > symbol is first encountered as a dependency of another symbol it will
    > > > be rechecked every time it is encountered until it's encountered at
    > > > the top level.
    > >
    > > You're correct, the check does too much.
    > >
    > > > Index: working-2.6/scripts/kconfig/symbol.c
    > > > ===================================================================
    > > > --- working-2.6.orig/scripts/kconfig/symbol.c 2005-10-20 12:40:45.000000000 +1000
    > > > +++ working-2.6/scripts/kconfig/symbol.c 2005-10-20 12:41:43.000000000 +1000
    > > > @@ -758,6 +758,8 @@
    > > > out:
    > > > if (sym2)
    > > > printf(" %s", sym->name);
    > > > + else
    > > > + sym->flags |= SYMBOL_CHECK_DONE;
    > > > sym->flags &= ~SYMBOL_CHECK;
    > > > return sym2;
    > > > }
    > >
    > > Actually this way it becomes redundant with SYMBOL_CHECKED, could you
    > > merge these two flags? The above check would be also probably better:
    >
    > Ok, done. There is now only SYMBOL_CHECKED (seemed a clearer name to
    > me), but it's semantics are like those of SYMBOL_CHECK_DONE were.
    >
    > > if (sym2) {
    > > printf(" %s", sym->name);
    > > if (sym2 == sym) {
    > > printf("\n");
    > > sym2 = NULL;
    > > }
    > > }
    > >
    > > So that this check will stop when it hits the start symbol and continue
    > > looking for more dependency problems, which is I think I intended with the
    > > original code.
    >
    > Erm.. ok. I don't entirely understand the intent of this is, but
    > applied anyway.
    >
    > > > Index: working-2.6/scripts/kconfig/zconf.y
    > > > ===================================================================
    > > > --- working-2.6.orig/scripts/kconfig/zconf.y 2005-10-20 12:40:45.000000000 +1000
    > > > +++ working-2.6/scripts/kconfig/zconf.y 2005-10-20 12:41:43.000000000 +1000
    > > > @@ -495,10 +495,9 @@
    > > > exit(1);
    > > > menu_finalize(&rootmenu);
    > > > for_all_symbols(i, sym) {
    > > > +/* fprintf(stderr, "Checking %s...\n", sym->name); */
    > >
    > > One "quilt refresh" missing? :-)
    >
    > Oops. Something like that.
    >
    > Oh.. one caveat, the diffs I have here to zconf.tab.c_shipped are
    > direct edits to match zconf.y - I didn't regenerate the file with
    > bison. I've done that to getting a whole lot of irrelevant changes in
    > the patch because I'm using a different version of bison to that used
    > for the existing zconf.tab.[ch]_shipped
    >
    > Anyway, revised version below:

    Now that 2.6.14 is out, do you intend to pass this patch on to Linus?

    -- 
    David Gibson			| I'll have my music baroque, and my code
    david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
    				| _way_ _around_!
    http://www.ozlabs.org/people/dgibson
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    

  • Next message: Dmitry Torokhov: "[GIT PULL] Tiny input update"

    Relevant Pages

    • Re: [PATCH] use new msleep() in ADT746x driver
      ... >>IMO this is moving the code away from what the coder appeared to intend. ... > This patch was done by the original author of the driver ... I think the author is making his driver less readable... ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • [PATCH] ppc32: rework l2 cache code
      ... This patch removes the code that tweaked the L1 cache when setting ... That was added a while ago in the intend of making ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [Lhms-devel] Re: 2.6.14-mm2
      ... >>Ah, okay. ... > didn't intend. ... In the patch I pointed to in the last mail, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [parisc-linux] Re: [PATCH 3/9] mm: parisc pte atomicity
      ... using your own tmpalias area sounds much better than getting ... I've simply not wrapped my head around the races, ... it looks like we agree that my patch is necessary and valid as is; ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: keyboard - was: Re: Linux 2.6.0-test4
      ... >> I was able to get the key unstuck by switching back and forth between ... I rebuild my kernel including your patch; ... I'll get back to you once I verify that the problem doesn't occur ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)