linux-kernel - Re: [PATCH v2] driver core: add wait event for deferred probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1360773717.1618.71.camel@anish-Inspiron-N5050>
Date:	Wed, 13 Feb 2013 22:11:57 +0530
From:	anish kumar <anish198519851985@...il.com>
To:	Haojian Zhuang <haojian.zhuang@...aro.org>
Cc:	gregkh@...uxfoundation.org, viro@...iv.linux.org.uk,
	rusty@...tcorp.com.au, hpa@...ux.intel.com, jim.cromie@...il.com,
	linux-kernel@...r.kernel.org, linux@....linux.org.uk,
	linus.walleij@...aro.org, grant.likely@...retlab.ca, arnd@...db.de,
	broonie@...nsource.wolfsonmicro.com, akpm@...ux-foundation.org,
	patches@...aro.org
Subject: Re: [PATCH v2] driver core: add wait event for deferred probe

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@...aro.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)


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