Q: nr_threads locking

Q: nr_threads locking

[Manfred Spraul]

Hi Andrew,

According to the comments, nr_threads is protected by lock_kernel, but 
do_fork() runs without the bkl for ages.
Would it be possible to use your percpu_counters for nr_threads? It 
seems to be used only to guard against fork bombs and for i_nlink of /proc.

--
    Manfred

Re: Q: nr_threads locking

[Andrew Morton]

Manfred Spraul <manfred@colorfullife.com> wrote:
>
> Hi Andrew,
> 
> According to the comments, nr_threads is protected by lock_kernel, but 
> do_fork() runs without the bkl for ages.
> Would it be possible to use your percpu_counters for nr_threads? It 
> seems to be used only to guard against fork bombs and for i_nlink of /proc.
> 

It would be possible, yes.

But thread creation is a "rare" event compared to pagefaults and syscalls. 
An atomic_t will be OK there.

Re: Q: nr_threads locking

[Manfred Spraul]

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

Andrew Morton wrote:

>It would be possible, yes.
>
>But thread creation is a "rare" event compared to pagefaults and syscalls. 
>An atomic_t will be OK there.
>  
>
Actually, the code is correct. The documentation it bogus. lock_kernel() 
never achieved any protection: the copy_xy() functions can sleep.

What about the attached docu update?

--
    Manfred

[-- Attachment #2: patch-nrthreads --]
[-- Type: text/plain, Size: 862 bytes --]

--- 2.5/kernel/fork.c	2003-04-20 11:19:14.000000000 +0200
+++ build-2.5/kernel/fork.c	2003-04-21 20:44:37.000000000 +0200
@@ -43,7 +43,9 @@
 extern int copy_semundo(unsigned long clone_flags, struct task_struct *tsk);
 extern void exit_semundo(struct task_struct *tsk);
 
-/* The idle threads do not count.. */
+/* The idle threads do not count..
+ * Protected by write_lock_irq(&tasklist_lock)
+ */
 int nr_threads;
 
 int max_threads;
@@ -792,9 +794,9 @@
 	atomic_inc(&p->user->processes);
 
 	/*
-	 * Counter increases are protected by
-	 * the kernel lock so nr_threads can't
-	 * increase under us (but it may decrease).
+	 * If multiple threads are within copy_process(), then this check
+	 * triggers too late. This doesn't hurt, the check is only there
+	 * to stop root fork bombs.
 	 */
 	if (nr_threads >= max_threads)
 		goto bad_fork_cleanup_count;