On 08/17, Luis R. Rodriguez wrote:
>
> In the last iteration that I have stress tested for corner cases I just
> get_task_struct() on the init and then put_task_struct() at the exit, is that
> fine too or are there reasons to prefer the module stuff?
I am fine either way.
I like the Takashi's idea because if sys_delete_module() is called before
initfn() completes it will return -EBUSY and not hang in TASK_UNINTERRUPTIBLE
state. But this is not necessarily good, so I leave this to you and Takashi.
> +/*
> + * Linux device drivers must strive to handle driver initialization
> + * within less than 30 seconds,
Well, perhaps the comment should name the reason ;)
> if device probing takes longer
> + * for whatever reason asynchronous probing of devices / loading
> + * firmware should be used. If a driver takes longer than 30 second
> + * on the initialization path
Or if the initialization code can't handle the errors properly (say,
mptsas can't handle the errors caused by SIGKILL).
> + * Drivers that use this helper should be considered broken and in need
> + * of some serious love.
> + */
Yes.
> +#define module_long_probe_init(initfn) \
> + static struct task_struct *__init_thread; \
> + static int _long_probe_##initfn(void *arg) \
> + { \
> + return initfn(); \
> + } \
> + static inline __init int __long_probe_##initfn(void) \
> + { \
> + __init_thread = kthread_create(_long_probe_##initfn,\
> + NULL, \
> + #initfn); \
> + if (IS_ERR(__init_thread)) \
> + return PTR_ERR(__init_thread); \
> + /* \
> + * callback won't check kthread_should_stop() \
> + * before bailing, so we need to protect it \
> + * before running it. \
> + */ \
> + get_task_struct(__init_thread); \
> + wake_up_process(__init_thread); \
> + return 0; \
> + } \
> + module_init(__long_probe_##initfn);
> +
> +/* To be used by modules that require module_long_probe_init() */
> +#define module_long_probe_exit(exitfn) \
> + static inline void __long_probe_##exitfn(void) \
> + { \
> + int err; \
> + /* \
> + * exitfn() will not be run if the driver's \
> + * real probe which is run on the kthread \
> + * failed for whatever reason, this will \
> + * wait for it to end. \
> + */ \
> + err = kthread_stop(__init_thread); \
> + if (!err) \
> + exitfn(); \
> + put_task_struct(__init_thread); \
> + } \
> + module_exit(__long_probe_##exitfn);
Both inline's look misleading, gcc will generate the code out-of-line
anyway. But this is cosmetic. And for cosmetic reasons, since the 1st
macro uses __init, the 2nd one should probably use __exit.
I believe this version is correct.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
