[PATCH 1/4] kthreads: move sched-realeted initialization from kthreadd context

[PATCH 1/4] kthreads: move sched-realeted initialization from kthreadd context

[Oleg Nesterov]

(on top of kthread-dont-looking-for-a-task-in-create_kthread-2.patch)

kthreadd is the single thread which implements ths "create" request,
move sched_setscheduler/etc from create_kthread() to kthread_create()
to improve the scalability.

We should be careful with sched_setscheduler(), use _nochek helper.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>

--- 6.29-rc3/kernel/kthread.c~1_SCHED	2009-01-30 09:40:12.000000000 +0100
+++ 6.29-rc3/kernel/kthread.c	2009-01-30 09:46:29.000000000 +0100
@@ -100,15 +100,7 @@ static void create_kthread(struct kthrea
 	if (pid < 0) {
 		create->result = ERR_PTR(pid);
 	} else {
-		struct sched_param param = { .sched_priority = 0 };
 		wait_for_completion(&create->started);
-		/*
-		 * root may have changed our (kthreadd's) priority or CPU mask.
-		 * The kernel thread should not inherit these properties.
-		 */
-		sched_setscheduler(create->result, SCHED_NORMAL, &param);
-		set_user_nice(create->result, KTHREAD_NICE_LEVEL);
-		set_cpus_allowed_ptr(create->result, CPU_MASK_ALL_PTR);
 	}
 	complete(&create->done);
 }
@@ -152,11 +144,20 @@ struct task_struct *kthread_create(int (
 	wait_for_completion(&create.done);
 
 	if (!IS_ERR(create.result)) {
+		struct sched_param param = { .sched_priority = 0 };
 		va_list args;
+
 		va_start(args, namefmt);
 		vsnprintf(create.result->comm, sizeof(create.result->comm),
 			  namefmt, args);
 		va_end(args);
+		/*
+		 * root may have changed our (kthreadd's) priority or CPU mask.
+		 * The kernel thread should not inherit these properties.
+		 */
+		sched_setscheduler_nocheck(create.result, SCHED_NORMAL, &param);
+		set_user_nice(create.result, KTHREAD_NICE_LEVEL);
+		set_cpus_allowed_ptr(create.result, CPU_MASK_ALL_PTR);
 	}
 	return create.result;
 }

Re: [PATCH 1/4] kthreads: move sched-realeted initialization from kthreadd context

[Rusty Russell]

On Friday 30 January 2009 23:03:50 Oleg Nesterov wrote:
> (on top of kthread-dont-looking-for-a-task-in-create_kthread-2.patch)

Hi Oleg,

   Thanks for the cc: Vitaliy, I never saw that patch.  I've included it in my queue now.

As to this patch, it seems marginal.  I've never been convinced that we should
be trying to rescue root if they choose to set kthreadd's prio anyway, but I'm also wondering why we care about kthread_create scalability!

Still, I'm happy to apply it with one change:

> +		/*
> +		 * root may have changed our (kthreadd's) priority or CPU mask.
> +		 * The kernel thread should not inherit these properties.
> +		 */
> +		sched_setscheduler_nocheck(create.result, SCHED_NORMAL, &param);
> +		set_user_nice(create.result, KTHREAD_NICE_LEVEL);
> +		set_cpus_allowed_ptr(create.result, CPU_MASK_ALL_PTR);

cpu_all_mask is the non-deprecated replacement for CPU_MASK_ALL_PTR.

Thanks!
Rusty.

Re: [PATCH 1/4] kthreads: move sched-realeted initialization from kthreadd context

[Oleg Nesterov]

On 01/31, Rusty Russell wrote:
>
> On Friday 30 January 2009 23:03:50 Oleg Nesterov wrote:
> > (on top of kthread-dont-looking-for-a-task-in-create_kthread-2.patch)
>
> Hi Oleg,
>
>    Thanks for the cc: Vitaliy, I never saw that patch.  I've included it in my queue now.
>
> As to this patch, it seems marginal.  I've never been convinced that we should
> be trying to rescue root if they choose to set kthreadd's prio anyway,

I agree. Personally, I never understood this too. But if we want to
remove this code we need the separate discussion, so I just moved it
to the caller's context.

> but I'm also wondering why we care about kthread_create scalability!

It is always better to speedup things and to decrease the latency ;)

But I also think this and the next patch make the code simpler and
more clean. Now the only thing create_kthread() does is a plain fork()
and nothing else, this is imho good.

> Still, I'm happy to apply it with one change:
>
> > +		/*
> > +		 * root may have changed our (kthreadd's) priority or CPU mask.
> > +		 * The kernel thread should not inherit these properties.
> > +		 */
> > +		sched_setscheduler_nocheck(create.result, SCHED_NORMAL, &param);
> > +		set_user_nice(create.result, KTHREAD_NICE_LEVEL);
> > +		set_cpus_allowed_ptr(create.result, CPU_MASK_ALL_PTR);
>
> cpu_all_mask is the non-deprecated replacement for CPU_MASK_ALL_PTR.

Great, thanks.

(btw, I thought about avoiding CPU_MASK_ALL_PTR too, but I didn't
 dare to mix 2 different things in one patch).

Oleg.