[PATCH, RFC] remove task_cache entirely

[PATCH, RFC] remove task_cache entirely

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

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



[-- Attachment #2: patch-remove-taskcache --]
[-- Type: text/plain, Size: 1741 bytes --]

--- 2.5/kernel/fork.c	2003-07-13 13:15:01.000000000 +0200
+++ build-2.5/kernel/fork.c	2003-07-13 13:14:00.000000000 +0200
@@ -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;

Re: [PATCH, RFC] remove task_cache entirely

[William Lee Irwin III]

On Sun, Jul 13, 2003 at 01:57:41PM +0200, Manfred Spraul wrote:
> 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


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
--- mm1-2.5.75-1/include/asm-i386/thread_info.h	2003-07-10 13:05:26.000000000 -0700
+++ mm1-2.5.75-stack-1/include/asm-i386/thread_info.h	2003-07-13 05:35:08.000000000 -0700
@@ -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)