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

Damn, sorry for noise ;)

I was going to suggest to introduce module_put_and_exit() to simplify
this and potentially other users, but it already exists. So this code
can use it too without additional complications.

On 08/17, Oleg Nesterov wrote:

On 08/17, Oleg Nesterov wrote:

quoted

On 08/17, Takashi Iwai wrote:

quoted

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.

quoted

		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.

Forgot to mention... and __long_probe_##initfn() could be simpler
without kthread_run,

	__init_thread = kthread_create(...);
	if (IS_ERR(__init_thread))
		return PTR_ERR();

	module_get(THIS_MODULE);
	wake_up_process(__init_thread);
	return 0;

but this is subjective, up to you.

Oleg.