Re: [PATCH v3 1/3] init / kthread: add module_long_probe_init() and module_long_probe_exit()

[Oleg Nesterov <oleg@redhat.com>]

On 08/17, Takashi Iwai wrote:
>
> How about just increasing/decreasing the module count for blocking the
> exit call?  For example:
>
> #define module_long_probe_init(initfn)				\
> 	static int _long_probe_##initfn(void *arg)		\
> 	{							\
> 		int ret = initfn();				\
> 		module_put(THIS_MODULE);			\

WINDOW, please see below.

> 		return ret;					\
> 	}							\
> 	static inline __init int __long_probe_##initfn(void)	\
> 	{							\
> 		struct task_struct *__init_thread;		\
> 		__module_get(THIS_MODULE);			\
> 		__init_thread = kthread_run(_long_probe_##initfn,\
> 					    NULL,		\
> 					    #initfn);		\
> 		if (IS_ERR(__init_thread)) {			\
> 			module_put(THIS_MODULE);		\
> 			return PTR_ERR(__init_thread);		\
> 		}						\
> 		return 0;					\
> 	}							\

I leave this to you and Luis, but personally I think this is very
nice idea, I like it. Because sys_delete_module() won't hang in D
state waiting for initfn().

There is a small problem. This module can be unloaded right after
module_put() above. In this case its memory can be unmapped and
the exiting thread can crash.

This is very unlikely, this thread needs to execute just a few insn
and escape from this module's memory. Given that only the buggy
modules should use this hack, perhaps we can even ignore this race.

But perhaps it makes sense to close this race anyway, and we already
have complete_and_exit() which can be used instead of "return ret"
above. Just we need the additional "static struct completion" and
module_exit() should call wait_for_completion.

Oleg.