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

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]

Date:	Mon, 8 Aug 2016 14:51:13 -0700
From:	Qing Huang <qing.huang@...cle.com>
To:	Frank Rowand <frowand.list@...il.com>, linux-kernel@...r.kernel.org
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...aro.org>,
	Santosh Shilimkar <santosh.shilimkar@...cle.com>
Subject: Re: [PATCH] device probe: add self triggered delayed work request



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@...uxfoundation.org>
>> Cc: Grant Likely<grant.likely@...aro.org>
>> Cc: Santosh Shilimkar<santosh.shilimkar@...cle.com>
>> Signed-off-by: Qing Huang<qing.huang@...cle.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
>>    *
>>