Re: [PATCH 1/8] mm,oom_reaper: Remove pointless kthread_run() failure check.

[Oleg Nesterov <oleg@redhat.com>]

I guess we misunderstood each other,

On 07/04, Tetsuo Handa wrote:
>
> Oleg Nesterov wrote:
> > On 07/04, Tetsuo Handa wrote:
> > >
> > > Oleg Nesterov wrote:
> > > > On 07/03, Tetsuo Handa wrote:
> > > > >
> > > > > If kthread_run() in oom_init() fails due to reasons other than OOM
> > > > > (e.g. no free pid is available), userspace processes won't be able to
> > > > > start as well.
> > > >
> > > > Why?
> > > >
> > > > The kernel will boot with or without your change, but

and yes, I probably confused you. I tried to say that in theory nothing
prevents the kernel from booting even if oom_init() fails.

> > IOW, this patch doesn't look correct without other changes?
>
> If you think that global init can successfully start after kthread_run()
> in oom_init() failed.

No, I think you are right. If kthread_run() fails at this stage than something
is seriously wrong anyway. And yes, for example hung_task_init() doesn't bother
to check the value returned by kthread_run().

I meant that this just looks wrong (and proc_dohung_task_timeout_secs() too).
And the warning can help to identify the problem.

In any case I agree, we should remove "oom_reaper_th",

>  static int __init oom_init(void)
>  {
> -	oom_reaper_th = kthread_run(oom_reaper, NULL, "oom_reaper");
> -	if (IS_ERR(oom_reaper_th)) {
> -		pr_err("Unable to start OOM reaper %ld. Continuing regardless\n",
> -				PTR_ERR(oom_reaper_th));
> -		oom_reaper_th = NULL;
> -	}
> +	struct task_struct *p = kthread_run(oom_reaper, NULL, "oom_reaper");
> +
> +	BUG_ON(IS_ERR(p));
>  	return 0;

Yes, do_initcall_level() ignores the error returned by fn(), so we need BUG()
or panic().

This is off-topic, but perhaps we should audit all initcalls and fix those
who return the error for no reason, say, bts_init(). And then change
do_one_initcall() to panic or at least WARN() if a non-modular initcall fails.

Oleg.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>