Re: [PATCH] Dynamic tick for x86 version 050609-2

From: Pavel Machek (pavel_at_ucw.cz)
Date: 06/10/05

  • Next message: Pavel Machek: "Re: [PATCH] Dynamic tick for x86 version 050602-2" 
    Date:	Fri, 10 Jun 2005 11:10:08 +0200
To: Tony Lindgren <tony@atomide.com>

    Hi!

    Some more nitpicking...

    > +/*
    > + * ---------------------------------------------------------------------------
    > + * Command line options
    > + * ---------------------------------------------------------------------------
    > + */
    > +static int __initdata dyntick_autoenable = 0;
    > +static int __initdata dyntick_useapic = 0;
    > +
    > +/*
    > + * dyntick=[enable|disable],[forceapic]
    > + */
    > +static int __init dyntick_setup(char *options)
    > +{
    > + if (!options)
    > + return 0;
    > +
    > + if (strstr(options, "enable"))
    > + dyntick_autoenable = 1;
    > +
    > + if (strstr(options, "forceapic"))
    > + dyntick_useapic = 1;
    > +
    > + return 0;
    > +}
    > +
    > +__setup("dyntick=", dyntick_setup);

    Well, your parsing is little too simplistic. If I pass
    dyntick=do_not_dare_to_enable_it, it still enables :-).

    > +/*
    > + * ---------------------------------------------------------------------------
    > + * Sysfs interface
    > + * ---------------------------------------------------------------------------
    > + */
    > +
    > +extern struct sys_device device_timer;
    > +
    > +static ssize_t show_dyn_tick_state(struct sys_device *dev, char *buf)
    > +{
    > + return sprintf(buf, "suitable:\t%i\n"
    > + "enabled:\t%i\n"
    > + "using APIC:\t%i\n",
    > + dyn_tick->state & DYN_TICK_SUITABLE,
    > + (dyn_tick->state & DYN_TICK_ENABLED) >> 1,
    > + (dyn_tick->state & DYN_TICK_USE_APIC) >> 3);

    You basically hardcode values of DYN_TICK_* here. Why not use !!() and
    loose dependency?

                                                                    Pavel
    -
    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: Pavel Machek: "Re: [PATCH] Dynamic tick for x86 version 050602-2"

    Relevant Pages

    • Re: [Kgdb-bugreport] [PATCH] Kill kgdb_serial
      ... It will split start_kernel command line parsing into early ... parse and late parse, but that's the price we have to pay to do special ... KGDB: Linux Kernel Source Level Debugger ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [Kgdb-bugreport] [PATCH] Kill kgdb_serial
      ... It will split start_kernel command line parsing into early ... > parse and late parse, but that's the price we have to pay to do special ... I think modifying the kernel to support this for kgdb is more like the tail ... 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/ ...
      (Linux-Kernel)
    • Re: module.viomap support for ppc64
      ... Hotplug still uses the map ... Parsing one big file will not improve performance, ... >> Noone cares about that old junk. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: 2.6.13-rc3 test: finding compile errors with make randconfig
      ... > I finally got around to parsing the results, top 40, sorted by name. ... > build script similar to: ... > any interesting errors I can find the particular config + error to recover ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • [PATCH][2.4.23-pre7] APICBASE fix backport from 2.6
      ... and we're not parsing any MP/ACPI ... tables, then we will map the wrong address, and ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)