Re: [PATCH] device probe: add self triggered delayed work request

[Frank Rowand <frowand.list@gmail.com>]

On 07/29/16 22:39, Qing Huang wrote:
> In normal condition, the device probe requests kept in deferred
> queue would only be triggered for re-probing when another new device
> probe is finished successfully. This change will set up a delayed
> trigger work request if the current deferred probe being added is
> the only one in the queue. This delayed work request will try to
> reactivate any device from the deferred queue for re-probing later.
> 
> By doing this, if the last device being probed in system boot process
> has a deferred probe error, this particular device will still be able
> to be probed again.

I am trying to understand the use case.

Can you explain the scenario you are trying to fix?  If I understand
correctly, you expect that something will change such that a later
probe attempt will succeed.  How will that change occur and why
will the deferred probe list not be processed in this case?

Why are you conditioning this on the deferred_probe_pending_list
being empty?

-Frank



> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Signed-off-by: Qing Huang <qing.huang@oracle.com>
> ---
>  drivers/base/dd.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 16688f5..251042d 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 deferred_trigger_count = ATOMIC_INIT(0);
> +static atomic_t deferred_self_trigger_count = ATOMIC_INIT(0);
> +#define DEFERRED_SELF_TRIGGER_INTERVAL 30 /* 30 secs */
> +#define DEFERRED_SELF_TRIGGER_ATTEMPTS 5  /* 5 times */
>  
>  /*
>   * In some cases, like suspend to RAM or hibernation, It might be reasonable
> @@ -115,10 +118,23 @@ static void deferred_probe_work_func(struct work_struct *work)
>  	mutex_unlock(&deferred_probe_mutex);
>  }
>  static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
> +void driver_deferred_probe_trigger_wrapper(struct work_struct *work);
> +static DECLARE_DELAYED_WORK(deferred_probe_trigger_work,
> +			    driver_deferred_probe_trigger_wrapper);
>  
>  static void driver_deferred_probe_add(struct device *dev)
>  {
> +	int local_self_trigger_count;
> +
>  	mutex_lock(&deferred_probe_mutex);
> +	local_self_trigger_count = atomic_read(&deferred_self_trigger_count);
> +	if (list_empty(&deferred_probe_pending_list) &&
> +	    local_self_trigger_count <= DEFERRED_SELF_TRIGGER_ATTEMPTS) {
> +		cancel_delayed_work(&deferred_probe_trigger_work);
> +		queue_delayed_work(deferred_wq, &deferred_probe_trigger_work,
> +				   HZ * DEFERRED_SELF_TRIGGER_INTERVAL);
> +	}
> +
>  	if (list_empty(&dev->p->deferred_probe)) {
>  		dev_dbg(dev, "Added to deferred list\n");
>  		list_add_tail(&dev->p->deferred_probe, &deferred_probe_pending_list);
> @@ -202,6 +218,15 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +/*
> + * Wrapper function for delayed trigger work requests
> + */
> +void driver_deferred_probe_trigger_wrapper(struct work_struct *work)
> +{
> +	atomic_inc(&deferred_self_trigger_count);
> +	driver_deferred_probe_trigger();
> +}
> +
>  /**
>   * deferred_probe_initcall() - Enable probing of deferred devices
>   *
>