Messages in this thread | | | Date | Fri, 10 Jun 2005 08:10:58 -0700 | From | Tony Lindgren <> | Subject | Re: [PATCH] Dynamic tick for x86 version 050609-2 |
| |
* Pavel Machek <pavel@ucw.cz> [050610 02:10]: > Hi! > > Some more nitpicking...
Great!
> > +/* > > + * --------------------------------------------------------------------------- > > + * 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 :-).
OK, I'll change that to test that enable is the first option.
> > +/* > > + * --------------------------------------------------------------------------- > > + * 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
OK, thanks.
Tony - 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/
| |