From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752753AbcHHVtl (ORCPT ); Mon, 8 Aug 2016 17:49:41 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:46004 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752426AbcHHVtk (ORCPT ); Mon, 8 Aug 2016 17:49:40 -0400 From: Qing Huang Subject: Re: [PATCH] device probe: add self triggered delayed work request To: Frank Rowand , linux-kernel@vger.kernel.org References: <1469857179-23863-1-git-send-email-qing.huang@oracle.com> <57A8EF35.2090903@gmail.com> Cc: Greg Kroah-Hartman , Grant Likely , Santosh Shilimkar Message-ID: <57A8FED1.7080003@oracle.com> Date: Mon, 8 Aug 2016 14:51:13 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <57A8EF35.2090903@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Cc: Grant Likely >> Cc: Santosh Shilimkar >> Signed-off-by: Qing Huang >> --- >> 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 >> * >>