Message ID | 1360637915-11212-1-git-send-email-haojian.zhuang@linaro.org |
---|---|
State | Changes Requested |
Headers | show |
On Tue, 2013-02-12 at 10:58 +0800, Haojian Zhuang wrote: > do_initcalls() could call all driver initialization code in kernel_init > thread. It means that probe() function will be also called from that > time. After this, kernel could access console & release __init section > in the same thread. > > But if device probe fails and moves into deferred probe list, a new > thread is created to reinvoke probe. If the device is serial console, > kernel has to open console failure & release __init section before > reinvoking failure. Because there's no synchronization mechanism. > Now add wait event to synchronize after do_initcalls(). Can you describe the problem you are facing in detail i.e. without this patch what is happening weird? > > Changelog: > v2: add comments on wait_for_device_probe(). > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > drivers/base/dd.c | 8 +++++--- > init/main.c | 2 ++ > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 65631015..23db672 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -53,6 +53,9 @@ static LIST_HEAD(deferred_probe_pending_list); > static LIST_HEAD(deferred_probe_active_list); > static struct workqueue_struct *deferred_wq; > > +static atomic_t probe_count = ATOMIC_INIT(0); > +static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); > + > /** > * deferred_probe_work_func() - Retry probing devices in the active list. > */ > @@ -77,6 +80,7 @@ static void deferred_probe_work_func(struct work_struct *work) > private = list_first_entry(&deferred_probe_active_list, > typeof(*dev->p), deferred_probe); > dev = private->device; > + atomic_dec(&probe_count); > list_del_init(&private->deferred_probe); > > get_device(dev); > @@ -257,9 +261,6 @@ int device_bind_driver(struct device *dev) > } > EXPORT_SYMBOL_GPL(device_bind_driver); > > -static atomic_t probe_count = ATOMIC_INIT(0); > -static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); > - > static int really_probe(struct device *dev, struct device_driver *drv) > { > int ret = 0; > @@ -308,6 +309,7 @@ probe_failed: > /* Driver requested deferred probing */ > dev_info(dev, "Driver %s requests probe deferral\n", drv->name); > driver_deferred_probe_add(dev); > + atomic_inc(&probe_count); > } else if (ret != -ENODEV && ret != -ENXIO) { > /* driver matched but the probe failed */ > printk(KERN_WARNING > diff --git a/init/main.c b/init/main.c > index 63534a1..141a6a4 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -786,6 +786,8 @@ static void __init do_basic_setup(void) > do_ctors(); > usermodehelper_enable(); > do_initcalls(); > + /* wait all deferred probe finished & prepare to drop __init section */ > + wait_for_device_probe(); static int __ref kernel_init(void *unused) { kernel_init_freeable(); /* need to finish all async __init code before freeing the memory */ async_synchronize_full(); Is not your wait_for_device_probe() also calling async_synchronize_full() function?AFAIK this function will send all the instructions before moving ahead no?My understanding originates from the below text. From kernel/async.c file Subsystem/driver initialization code that scheduled asynchronous probe functions, but which shares global resources with other drivers/subsystems that do not use the asynchronous call feature, need to do a full synchronization with the async_synchronize_full() function, before returning from their init function. This is to maintain strict ordering between the asynchronous and synchronous parts of the kernel. > } > > static void __init do_pre_smp_initcalls(void)
On Tue, 12 Feb 2013 10:58:35 +0800, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: > do_initcalls() could call all driver initialization code in kernel_init > thread. It means that probe() function will be also called from that > time. After this, kernel could access console & release __init section > in the same thread. > > But if device probe fails and moves into deferred probe list, a new > thread is created to reinvoke probe. If the device is serial console, > kernel has to open console failure & release __init section before > reinvoking failure. Because there's no synchronization mechanism. > Now add wait event to synchronize after do_initcalls(). > > Changelog: > v2: add comments on wait_for_device_probe(). > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> As replied to on v1 of your patch, this change will cause a deadlock. The following change would do the same without the deadlock issue: static int deferred_probe_initcall(void) { deferred_wq = create_singlethread_workqueue("deferwq"); if (WARN_ON(!deferred_wq)) return -ENOMEM; driver_deferred_probe_enable = true; + deferred_probe_work_func(NULL); - driver_deferred_probe_trigger(); return 0; } late_initcall(deferred_probe_initcall); g.
diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 65631015..23db672 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -53,6 +53,9 @@ static LIST_HEAD(deferred_probe_pending_list); static LIST_HEAD(deferred_probe_active_list); static struct workqueue_struct *deferred_wq; +static atomic_t probe_count = ATOMIC_INIT(0); +static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); + /** * deferred_probe_work_func() - Retry probing devices in the active list. */ @@ -77,6 +80,7 @@ static void deferred_probe_work_func(struct work_struct *work) private = list_first_entry(&deferred_probe_active_list, typeof(*dev->p), deferred_probe); dev = private->device; + atomic_dec(&probe_count); list_del_init(&private->deferred_probe); get_device(dev); @@ -257,9 +261,6 @@ int device_bind_driver(struct device *dev) } EXPORT_SYMBOL_GPL(device_bind_driver); -static atomic_t probe_count = ATOMIC_INIT(0); -static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue); - static int really_probe(struct device *dev, struct device_driver *drv) { int ret = 0; @@ -308,6 +309,7 @@ probe_failed: /* Driver requested deferred probing */ dev_info(dev, "Driver %s requests probe deferral\n", drv->name); driver_deferred_probe_add(dev); + atomic_inc(&probe_count); } else if (ret != -ENODEV && ret != -ENXIO) { /* driver matched but the probe failed */ printk(KERN_WARNING diff --git a/init/main.c b/init/main.c index 63534a1..141a6a4 100644 --- a/init/main.c +++ b/init/main.c @@ -786,6 +786,8 @@ static void __init do_basic_setup(void) do_ctors(); usermodehelper_enable(); do_initcalls(); + /* wait all deferred probe finished & prepare to drop __init section */ + wait_for_device_probe(); } static void __init do_pre_smp_initcalls(void)
do_initcalls() could call all driver initialization code in kernel_init thread. It means that probe() function will be also called from that time. After this, kernel could access console & release __init section in the same thread. But if device probe fails and moves into deferred probe list, a new thread is created to reinvoke probe. If the device is serial console, kernel has to open console failure & release __init section before reinvoking failure. Because there's no synchronization mechanism. Now add wait event to synchronize after do_initcalls(). Changelog: v2: add comments on wait_for_device_probe(). Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- drivers/base/dd.c | 8 +++++--- init/main.c | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-)