Re: [PATCH] 2.5.63 tsk->usage count.

Re: [PATCH] 2.5.63 tsk->usage count.

[Martin Schwidefsky]

> This actually looks wrong, it ends up doing free_user() twice because a
> final put_task_struct() does that too these days.
>
> Does this alternate patch work for you instead?

Works fine. I had a "Badness in __put_task_struct at kernel/fork.c:77"
from time to time. I couldn't reproduce this with your patch.

blue skies,
   Martin

[PATCH] 2.5.63 tsk->usage count.

[Martin Schwidefsky]

Hi Linus,
while debugging a memory leak with task structures on s390
I found something related to it. If copy_process fails for some
reason the task structure created with dup_task_struct has set
p->usage to 2 but only one put_task_struct is done in the error
cleanup code. The attached patch should take care of it.

blue skies,
  Martin.

diff -urN linux-2.5/kernel/fork.c linux-2.5-fork/kernel/fork.c
--- linux-2.5/kernel/fork.c	Thu Feb 27 11:25:56 2003
+++ linux-2.5-fork/kernel/fork.c	Thu Feb 27 10:11:42 2003
@@ -1034,6 +1034,8 @@
 	atomic_dec(&p->user->processes);
 	free_uid(p->user);
 bad_fork_free:
+	/* dup_task_struct sets p->usage to 2 */
+	atomic_dec(&p->usage);
 	put_task_struct(p);
 	goto fork_out;
 }

Re: [PATCH] 2.5.63 tsk->usage count.

[Linus Torvalds]

On Thu, 27 Feb 2003, Martin Schwidefsky wrote:
> while debugging a memory leak with task structures on s390
> I found something related to it. If copy_process fails for some
> reason the task structure created with dup_task_struct has set
> p->usage to 2 but only one put_task_struct is done in the error
> cleanup code. The attached patch should take care of it.

This actually looks wrong, it ends up doing free_user() twice because a 
final put_task_struct() does that too these days.

Does this alternate patch work for you instead?

		Linus

----
===== kernel/fork.c 1.110 vs edited =====
--- 1.110/kernel/fork.c	Tue Feb 25 02:50:01 2003
+++ edited/kernel/fork.c	Thu Feb 27 22:56:36 2003
@@ -72,15 +72,8 @@
 	return total;
 }
 
-void __put_task_struct(struct task_struct *tsk)
+static void free_task_struct(struct task_struct *tsk)
 {
-	WARN_ON(!(tsk->state & (TASK_DEAD | TASK_ZOMBIE)));
-	WARN_ON(atomic_read(&tsk->usage));
-	WARN_ON(tsk == current);
-
-	security_task_free(tsk);
-	free_uid(tsk->user);
-
 	/*
 	 * The task cache is effectively disabled right now.
 	 * Do we want it? The slab cache already has per-cpu
@@ -103,6 +96,17 @@
 	}
 }
 
+void __put_task_struct(struct task_struct *tsk)
+{
+	WARN_ON(!(tsk->state & (TASK_DEAD | TASK_ZOMBIE)));
+	WARN_ON(atomic_read(&tsk->usage));
+	WARN_ON(tsk == current);
+
+	security_task_free(tsk);
+	free_uid(tsk->user);
+	free_task_struct(tsk);
+}
+
 void add_wait_queue(wait_queue_head_t *q, wait_queue_t * wait)
 {
 	unsigned long flags;
@@ -1034,7 +1038,7 @@
 	atomic_dec(&p->user->processes);
 	free_uid(p->user);
 bad_fork_free:
-	put_task_struct(p);
+	free_task_struct(p);
 	goto fork_out;
 }