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

[Qing Huang]

On 08/08/2016 01:44 PM, Frank Rowand wrote:
> 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

It turns out one corner case which we worried about has already been 
solved in the really_probe() function by comparing 
'deferred_trigger_count' values.

Another use case we are investigating now: when we probe a device, the 
main thread returns EPROBE_DEFER from the driver after we spawn a child 
thread to do the actual init work. So we can initialize multiple similar 
devices at the same time. After the child thread finishes its task, we 
can call driver_deferred_probe_trigger() directly from child thread to 
re-probe the device(driver_deferred_probe_trigger() has to be exported 
though). Or we could rely on something in this patch to re-probe the 
deferred devices from the pending list...

What do you suggest?

Thanks,
-Qing

> > Cc: Greg Kroah-Hartman<gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Grant Likely<grant.likely@xxxxxxxxxx>
> > Cc: Santosh Shilimkar<santosh.shilimkar@xxxxxxxxxx>
> > Signed-off-by: Qing Huang<qing.huang@xxxxxxxxxx>
> > ---
> >   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
> >    *