lkml.org 
[lkml]   [2004]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] cpu hotplug fix
From
Date
On Sat, 2004-03-20 at 18:40, Christoph Hellwig wrote:
> On Sat, Mar 20, 2004 at 05:36:42PM +1100, Anton Blanchard wrote:
> >
> > start_cpu_timer was changed to __init a few hours before the cpu hotplug
> > patches went in. With cpu hotplug it can be called at any time, so fix
> > this.
>
> So we're wasting memory due to the few machines supporting hot-plug cpus
> in slab now? What about adding __cpuinit ala __devinit instead?

Just use __devinit.

More importantly, Christoph, you're missing the forest through the
trees: what the hell is start_cpu_timer trying to do?

Name: Remove Strange start_cpu_timer Code
Status: Untested

Why is start_cpu_timer checking for fn being NULL? And why isn't
every CPU's reap_timer initialized in cpucache_init (which is an
__init function).

Clean up this mess by just starting the timer in start_cpu_timer.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .15051-linux-2.6.5-rc2/mm/slab.c .15051-linux-2.6.5-rc2.updated/mm/slab.c
--- .15051-linux-2.6.5-rc2/mm/slab.c 2004-03-20 21:21:33.000000000 +1100
+++ .15051-linux-2.6.5-rc2.updated/mm/slab.c 2004-03-20 21:45:06.000000000 +1100
@@ -576,17 +576,12 @@ static void __slab_error(const char *fun
* Add the CPU number into the expiry time to minimize the possibility of the
* CPUs getting into lockstep and contending for the global cache chain lock.
*/
-static void __init start_cpu_timer(int cpu)
+static inline void start_cpu_timer(int cpu)
{
struct timer_list *rt = &per_cpu(reap_timers, cpu);

- if (rt->function == NULL) {
- init_timer(rt);
- rt->expires = jiffies + HZ + 3*cpu;
- rt->data = cpu;
- rt->function = reap_timer_fnc;
- add_timer_on(rt, cpu);
- }
+ rt->expires = jiffies + HZ + 3*cpu;
+ add_timer_on(rt, cpu);
}

#ifdef CONFIG_HOTPLUG_CPU
@@ -798,7 +793,13 @@ int __init cpucache_init(void)
* Register the timers that return unneeded
* pages to gfp.
*/
- for (cpu = 0; cpu < NR_CPUS; cpu++) {
+ for_each_cpu(cpu) {
+ struct timer_list *rt = &per_cpu(reap_timers, cpu);
+
+ init_timer(rt);
+ rt->data = cpu;
+ rt->function = reap_timer_fnc;
+
if (cpu_online(cpu))
start_cpu_timer(cpu);
}


--
Anyone who quotes me in their signature is an idiot -- Rusty Russell

-
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/

\
 
 \ /
  Last update: 2005-03-22 14:01    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site