Skip to contentSkip to conversation

remove task_cache entirely

RFC PATCH In review Manfred Spraul 2 participants 2 messages Jul 13, 2003

Subsystems (3)
MM core Scheduler exec and binfmt
lore
Share & tools
lore

Manfred Spraul

author
#0 lore raw

RFC PATCH 1/1 remove task_cache entirely

kernel/fork.c contains a disabled cache for task stuctures. task structures are placed into the task cache only if "tsk==current", and "tsk==current" is impossible. There is even a WARN_ON against that in __put_task_struct().

What should we do with it? I would remove it entirely - it's dead code. Any objections?

One problem is that order-1 allocations are not cached per-cpu - what about using kmem_cache_alloc for the stack?

--
Manfred

kernel/fork.c +11 -43
@@ -53,13 +53,6 @@
 
 rwlock_t tasklist_lock __cacheline_aligned = RW_LOCK_UNLOCKED;  /* outer */
 
-/*
- * A per-CPU task cache - this relies on the fact that
- * the very last portion of sys_exit() is executed with
- * preemption turned off.
- */
-static task_t *task_cache[NR_CPUS] __cacheline_aligned;
-
 int nr_processes(void)
 {
 	int cpu;

@@ -80,26 +73,8 @@
 
 static void free_task(struct task_struct *tsk)
 {
-	/*
-	 * The task cache is effectively disabled right now.
-	 * Do we want it? The slab cache already has per-cpu
-	 * stuff, but the thread info (usually a order-1 page
-	 * allocation) doesn't.
-	 */
-	if (tsk != current) {
-		free_thread_info(tsk->thread_info);
-		free_task_struct(tsk);
-	} else {
-		int cpu = get_cpu();
-
-		tsk = task_cache[cpu];
-		if (tsk) {
-			free_thread_info(tsk->thread_info);
-			free_task_struct(tsk);
-		}
-		task_cache[cpu] = current;
-		put_cpu();
-	}
+	free_thread_info(tsk->thread_info);
+	free_task_struct(tsk);
 }
 
 void __put_task_struct(struct task_struct *tsk)

@@ -220,25 +195,18 @@
 {
 	struct task_struct *tsk;
 	struct thread_info *ti;
-	int cpu = get_cpu();
 
 	prepare_to_copy(orig);
 
-	tsk = task_cache[cpu];
-	task_cache[cpu] = NULL;
-	put_cpu();
-	if (!tsk) {
-		tsk = alloc_task_struct();
-		if (!tsk)
-			return NULL;
-
-		ti = alloc_thread_info(tsk);
-		if (!ti) {
-			free_task_struct(tsk);
-			return NULL;
-		}
-	} else
-		ti = tsk->thread_info;
+	tsk = alloc_task_struct();
+	if (!tsk)
+		return NULL;
+
+	ti = alloc_thread_info(tsk);
+	if (!ti) {
+		free_task_struct(tsk);
+		return NULL;
+	}
 
 	*ti = *orig->thread_info;
 	*tsk = *orig;
#1 lore raw
Quoted 8 lines · Manfred Spraul

kernel/fork.c contains a disabled cache for task stuctures. task structures are placed into the task cache only if "tsk==current", and "tsk==current" is impossible. There is even a WARN_ON against that in __put_task_struct().
What should we do with it? I would remove it entirely - it's dead code. Any objections?
One problem is that order-1 allocations are not cached per-cpu - what about using kmem_cache_alloc for the stack?

I've been slab allocating the stack on i386 for some time, and it has gone without incident in pgcl, -wli, -mjb (?), and so on. kmalloc() is fine; there isn't any particularly compelling reason for a dedicated slab as there's no preconstruction to do, though it can be arranged.

Basically, it works, there's no obvious reason not to, and (even better) it's not totally invisible to the VM and even makes overhead reportable.

akpm, this is a two line change with almost no effect apart from theoretically improved SMP efficiency and more accurate reporting. Shall we?

-- wli

include/asm-i386/thread_info.h +2 -2
diff -prauN mm1-2.5.75-1/include/asm-i386/thread_info.h mm1-2.5.75-stack-1/include/asm-i386/thread_info.h
@@ -87,8 +87,8 @@ static inline struct thread_info *curren
 
 /* thread information allocation */
 #define THREAD_SIZE (2*PAGE_SIZE)
-#define alloc_thread_info(tsk) ((struct thread_info *) __get_free_pages(GFP_KERNEL,1))
-#define free_thread_info(ti) free_pages((unsigned long) (ti), 1)
+#define alloc_thread_info(task) ((struct thread_info *)kmalloc(THREAD_SIZE, GFP_KERNEL))
+#define free_thread_info(info)	kfree(info)
 #define get_thread_info(ti) get_task_struct((ti)->task)
 #define put_thread_info(ti) put_task_struct((ti)->task)