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

[Greg KH]

On Wed, Aug 13, 2014 at 07:59:06AM +0900, Tetsuo Handa wrote:
> Luis R. Rodriguez wrote:
> > Tetsuo bisected and found that commit 786235ee \"kthread: make
> > kthread_create() killable\" modified kthread_create() to bail as
> > soon as SIGKILL is received.
> 
> I just wrote commit 786235ee. It is not Tetsuo who bisected it.
> 
> > @@ -128,4 +129,38 @@ bool queue_kthread_work(struct kthread_worker *worker,
> > void flush_kthread_work(struct kthread_work *work);
> > void flush_kthread_worker(struct kthread_worker *worker);
> > 
> > +#ifndef MODULE
> > +
> > +#define module_long_probe_init(x)      __initcall(x);
> > +#define module_long_probe_exit(x)      __exitcall(x);
> > +
> > +#else
> > +/* To be used by modules which can take over 30 seconds at probe */
> > +#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_run(_long_probe_##initfn,\\
> > +                             NULL,          \\
> > +                             #initfn);          \\
> > +          if (IS_ERR(__init_thread))               \\
> > +               return PTR_ERR(__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)          \\
> > +     {                                   \\
> > +          exitfn();                         \\
> 
> exitfn() must not be called if initfn() failed or has not
> completed yet. You need a bool variable for indicating that
> we are ready to call exitfn().
> 
> Also, subsequent userspace operations may fail if
> we return to userspace before initfn() completes
> (e.g. device nodes are not created yet).

I doubt that this will be a problem, as device nodes are usually created
_after_ module_init() returns.

But the cleanup issues are real on error paths.  Given that these
drivers will need "work" anyway, I don't think it's really a big deal.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/