Re: [PATCH] i386 No-Idle-Hz aka Dynamic-Ticks 4

Adrian Bunk
2005-08-06T17:47:49Z

On Sat, Aug 06, 2005 at 02:39:40AM +1000, Con Kolivas wrote:

> Here's my most current version of the dynamic ticks patch for i386 with some
> more minor cleanups already discussed and cosmetic changes ( also available
> at http://ck.kolivas.org/patches/dyn-ticks/ ).
>
> Cheers,
> Con
>...
> --- linux-2.6.13-rc5-ck2.orig/arch/i386/kernel/dyn-tick.c 2005-01-12 16:19:45.000000000 +1100
> +++ linux-2.6.13-rc5-ck2/arch/i386/kernel/dyn-tick.c 2005-08-05 20:48:57.000000000 +1000
>...
> +void arch_reprogram_timer(void)
> +{

static?

>...
> +int __init dyn_tick_init(void)
> +{

static?

>...
> +irqreturn_t dyn_tick_timer_interrupt(int irq, void *dev_id,
> + struct pt_regs *regs)
> +{

static?

>...
> +int __init dyn_tick_arch_init(void)
> +{

static?

>...
> +/* Functions that need blank prototypes for !CONFIG_NO_IDLE_HZ below here */
> +inline void set_dyn_tick_max_skip(u32 apic_timer_val)
> +{
> + dyn_tick->max_skip = 0xffffff / apic_timer_val;
> +}
> +
> +inline void setup_dyn_tick_use_apic(unsigned int calibration_result)
> +{
> + if (calibration_result)
> + dyn_tick->state |= DYN_TICK_USE_APIC;
> + else
> + printk(KERN_INFO "dyn-tick: Cannot use local APIC\n");
> +}

You can drop the "inline" on both functions (it doesn't make any
practical difference since they are only called from other files).

>...
> --- linux-2.6.13-rc5-ck2.orig/include/asm-i386/dyn-tick.h 2005-01-12 16:19:45.000000000 +1100
> +++ linux-2.6.13-rc5-ck2/include/asm-i386/dyn-tick.h 2005-08-05 20:57:02.000000000 +1000
>...
> +#if defined(CONFIG_DYN_TICK_USE_APIC) && \
> + (defined(CONFIG_SMP) || defined(CONFIG_X86_UP_APIC))
> +#define cpu_has_local_apic() (dyn_tick->state & DYN_TICK_USE_APIC)
> +#else
> +#define cpu_has_local_apic() 0
> +#endif
> +
> +#if defined(CONFIG_DYN_TICK_USE_APIC)
> +#define dyntick_apicable() 1
> +#else
> +#define dyntick_apicable() 0
> +#endif

Constants that mimic functions aren't good coding style.

Please make them either uppercase #define's without function braces or
"static inline" functions.

>...
> --- linux-2.6.13-rc5-ck2.orig/kernel/dyn-tick.c 2005-01-12 16:19:45.000000000 +1100
> +++ linux-2.6.13-rc5-ck2/kernel/dyn-tick.c 2005-08-05 21:02:44.000000000 +1000
>...
> +struct dyn_tick_state dyn_tick_state;

static?

>...
> +struct dyn_tick_timer *dyn_tick_cfg;
>...

static?

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed