Prasad Sodagudi <psoda...@codeaurora.org> writes:
> To make all cpu unbound deferrable timers are scalable, introduce a common
> timer base which is only for cpu unbound deferrable timers to make those
> are indeed cpu unbound so that can be scheduled by any of non idle cpus.
> This common timer fixes scalability issue of delayed work and all other cpu
> unbound deferrable timer using implementations.
Scalability? That's really the wrong term here. A global timer base is
the opposite and you really want to explain why this is not creating a
scalability problem on large systems.
> #ifdef CONFIG_SMP
> +struct timer_base timer_base_deferrable;
> unsigned int sysctl_timer_migration = 1;
>
> DEFINE_STATIC_KEY_FALSE(timers_migration_enabled);
> @@ -841,8 +842,14 @@ static inline struct timer_base *get_timer_cpu_base(u32
> tflags, u32 cpu)
> * If the timer is deferrable and NO_HZ_COMMON is set then we need
> * to use the deferrable base.
> */
> - if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && (tflags & TIMER_DEFERRABLE))
> - base = per_cpu_ptr(&timer_bases[BASE_DEF], cpu);
> + if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && (tflags & TIMER_DEFERRABLE)) {
> +#ifdef CONFIG_SMP
> + base = &timer_base_deferrable;
> +#endif
There are definitely smarter ways of solving this than sprinkling
#ifdef's around the code.
> + if (tflags & TIMER_PINNED)
> + base = per_cpu_ptr(&timer_bases[BASE_DEF], cpu);
> + }
> +
> return base;
> }
> @@ -1785,8 +1798,14 @@ static __latent_entropy void run_timer_softirq(struct
> softirq_action *h)
> struct timer_base *base = this_cpu_ptr(&timer_bases[BASE_STD]);
>
> __run_timers(base);
> - if (IS_ENABLED(CONFIG_NO_HZ_COMMON))
> + if (IS_ENABLED(CONFIG_NO_HZ_COMMON)) {
> __run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));
> +#ifdef CONFIG_SMP
> + if (tick_do_timer_cpu == TICK_DO_TIMER_NONE ||
> + tick_do_timer_cpu == smp_processor_id())
> + __run_timers(&timer_base_deferrable);
> +#endif
Again, this can be solved in readable ways. Just slapping #ifdefs all
over the place is sloppy and lazy.
Aside of that accessing the tick internals here open coded is just a
layering violation.
> + }
> }
>
> /*
> @@ -2025,6 +2044,16 @@ static void __init init_timer_cpu(int cpu)
> }
> }
>
> +#if defined(CONFIG_NO_HZ_COMMON) && defined(CONFIG_SMP)
> +static void __init init_timer_deferrable_global(void)
> +{
> + timer_base_deferrable.cpu = nr_cpu_ids;
This was obviously never tested with CONFIG_DEBUG_PER_CPU_MAPS=y as this
will simply result in out of bounds accesses.
> static void __init init_timer_cpus(void)
> {
> int cpu;
> @@ -2036,6 +2065,9 @@ static void __init init_timer_cpus(void)
> void __init init_timers(void)
> {
> init_timer_cpus();
> +#if defined(CONFIG_NO_HZ_COMMON) && defined(CONFIG_SMP)
> + init_timer_deferrable_global();
> +#endif
Stub functions exist to avoid this unreadable #ifdef garbage.
Thanks,
tglx
