From patchwork Thu Oct 22 15:12:34 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grygorii Strashko X-Patchwork-Id: 7465411 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A756BBEEA4 for ; Thu, 22 Oct 2015 15:14:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E079120860 for ; Thu, 22 Oct 2015 15:14:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CDCF5208C0 for ; Thu, 22 Oct 2015 15:14:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964995AbbJVPOB (ORCPT ); Thu, 22 Oct 2015 11:14:01 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:54426 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756613AbbJVPNz (ORCPT ); Thu, 22 Oct 2015 11:13:55 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id t9MFCjH5013014; Thu, 22 Oct 2015 10:12:45 -0500 Received: from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id t9MFCi2A006744; Thu, 22 Oct 2015 10:12:44 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.224.2; Thu, 22 Oct 2015 10:12:45 -0500 Received: from [172.22.234.158] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id t9MFCZ1G015785; Thu, 22 Oct 2015 10:12:36 -0500 Subject: Re: Alternative approach to solve the deferred probe To: Russell King - ARM Linux References: <20151019153548.GM32532@n2100.arm.linux.org.uk> <20151020154656.GY32532@n2100.arm.linux.org.uk> <56270D5B.5010902@gmail.com> <20151021081847.GB32532@n2100.arm.linux.org.uk> <5627B0F7.7010600@gmail.com> <5627C381.60602@ti.com> <20151021172042.GJ32532@n2100.arm.linux.org.uk> <5627D5DC.1040708@ti.com> <20151021182817.GM32532@n2100.arm.linux.org.uk> CC: , Geert Uytterhoeven , Tomeu Vizoso , Mark Brown , Greg Kroah-Hartman , Rob Herring , Michael Turquette , Stephen Boyd , Vinod Koul , Dan Williams , Linus Walleij , Alexandre Courbot , Thierry Reding , David Airlie , =?UTF-8?Q?Terje_Bergstr=c3=b6m?= , Stephen Warren , Wolfram Sang , Grant Likely , Kishon Vijay Abraham I , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Liam Girdwood , Felipe Balbi , Jingoo Han , Lee Jones , Jean-Christophe Plagniol-Villard , Tomi Valkeinen , "linux-kernel@vger.kernel.org" , linux-clk , , "linux-gpio@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "linux-tegra@vger.kernel.org" , Linux I2C , "devicetree@vger.kernel.org" , "linux-pm@vger.kernel.org" , Linux PWM List , "linux-usb@vger.kernel.org" , Linux Fbdev development list From: Grygorii Strashko Message-ID: <5628FCE2.1070409@ti.com> Date: Thu, 22 Oct 2015 18:12:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151021182817.GM32532@n2100.arm.linux.org.uk> Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, On 10/21/2015 09:28 PM, Russell King - ARM Linux wrote: > On Wed, Oct 21, 2015 at 09:13:48PM +0300, Grygorii Strashko wrote: >> But I worry a bit (and that my main point) about these few additional >> rounds of deferred device probing which I have right now and which allows >> some of drivers to finish, finally, their probes successfully. >> With proposed change I'll get more messages in boot log, but some of >> them will belong to drivers which have been probed successfully and so, >> they will be not really useful. > > Then you haven't properly understood my proposal. > > I want to get rid of all the "X deferred its probing" messages up until > the point that we set the "please report deferred probes" flag. > > That _should_ mean that all the deferred probing that goes on becomes > _totally_ silent and becomes hidden (unless you really want to see it, > in which case we can make a debug option which turns it on) up until > we're at the point where we want to enter userspace. > > At that point, we then report into the kernel log which devices are > still deferring and, via appropriately placed dev_warn_deferred(), > the reasons why the devices are being deferred. > > So, gone will be all the messages earlier in the log about device X > not having a GPIO/clock/whatever because the device providing the > GPIO/clock/whatever hasn't been probed. > > If everything is satisfied by the time we run this last round (again, > I'm not using a three line sentence to describe exactly what I mean, > I'm sure you know by now... oops, I just did) then the kernel will > report nothing about any deferrals. That's _got_ to be an improvement. Sorry Master, but you really don't need to spend so much time typing the same things three times - I understand what are you trying to do :( I did my comments with assumption that it's not officially prohibited/deprecated to register drivers (and execute probes) from late_initcall() layer (just recommended) and there are still bunch of drivers which are doing this. Now I see that it's not a recommendation any more, and deferred_probe_initcall() might be a good place to activate driver_deferred_probe_report if goal is to identify and fix such drivers. Sorry for your time. > >> >> As result, I think, the most important thing is to identify (or create) >> some point during kernel boot when it will be possible to say that all >> built-in drivers (at least) finish their probes 100% (done or defer). >> >> Might be do_initcalls() can be updated (smth like this): >> static void __init do_initcalls(void) >> { >> int level; >> >> for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++) >> do_initcall_level(level); >> >> + wait_for_device_probe(); >> + /* Now one final round, reporting any devices that remain deferred */ >> + driver_deferred_probe_report = true; >> + driver_deferred_probe_trigger(); >> + wait_for_device_probe(); >> } >> >> Also, in my opinion, it will be useful if this debugging feature will be >> optional. > > I wonder why you want it optional... so I'm going to guess and cover > both cases I can think of below to head off another round of reply on > this point (sorry if this sucks eggs.) > > I don't see it as being optional, because it's going to be cheap to run > in the case of a system which has very few or no errors - which is what > you should have for production systems, right? > Also, I've spend some time today testing your proposal - hope you'll find results useful. I've applied truncated version of your patch (diff below) on TI's 4.1 kernel and run few tests (log is below) on dra7-evm/am43xx-gpevm - K4.1 is not far away from LKML, so I assume this test is valid. Overall boot process consists from two stages: kernel boot and modules loading. My Changes: - only really_probe() modified to show deferred device/drivers From the log I can see additional messages in log when modules are loading, because driver_deferred_probe_report is still true - dwc3 probes were deferred, but then finally succeeded. So, as you've mentioned, it seems a good thing to deactivate driver_deferred_probe_report and provide user with ability to turn it on again (and probably re-trigger deferred device probing). I've found no issues during Kernel boot (built-in) time, new messages are displayed only if probe is failed for some drivers: [ 3.219700] ====================================== deferred_probe_initcalll [ 3.226820] platform omapdrm.0: Driver omapdrm requests probe deferral [ 3.233378] platform omapdrm.0: deferring probe: ==== Driver omapdrm requests probe deferral [ 3.242084] dra7evm-tpd12s015 encoder@1: failed to parse CT CP HPD gpio [ 3.248737] platform encoder@1: Driver dra7evm-tpd12s015 requests probe deferral [ 3.256168] platform encoder@1: deferring probe: ==== Driver dra7evm-tpd12s015 requests probe deferral [ 3.265763] connector-hdmi connector@1: failed to find video source [ 3.272067] platform connector@1: Driver connector-hdmi requests probe deferral [ 3.279410] platform connector@1: deferring probe: ==== Driver connector-hdmi requests probe deferral ^^ above drivers will be deferred forever Thanks. diff --git a/drivers/base/dd.c b/drivers/base/dd.c index e7d2545..d61fa47 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -129,6 +129,27 @@ void driver_deferred_probe_del(struct device *dev) mutex_unlock(&deferred_probe_mutex); } +static bool driver_deferred_probe_report; + +/** + * dev_warn_deferred() - report why a probe has been deferred + */ +void dev_warn_deferred(struct device *dev, const char *fmt, ...) +{ + if (driver_deferred_probe_report) { + struct va_format vaf; + va_list ap; + + va_start(ap, fmt); + vaf.fmt = fmt; + vaf.va = ≈ + + dev_err(dev, "deferring probe: %pV", &vaf); + va_end(ap); + } +} +EXPORT_SYMBOL_GPL(dev_warn_deferred); + static bool driver_deferred_probe_enable = false; /** * driver_deferred_probe_trigger() - Kick off re-probing deferred devices @@ -188,6 +209,15 @@ static int deferred_probe_initcall(void) driver_deferred_probe_trigger(); /* Sort as many dependencies as possible before exiting initcalls */ flush_workqueue(deferred_wq); + + pr_err("====================================== deferred_probe_initcalll\n"); + + /* Now one final round, reporting any devices that remain deferred */ + driver_deferred_probe_report = true; + driver_deferred_probe_trigger(); + /* Sort as many dependencies as possible before exiting initcalls */ + flush_workqueue(deferred_wq); + return 0; } late_initcall(deferred_probe_initcall); @@ -342,7 +372,8 @@ probe_failed: switch (ret) { case -EPROBE_DEFER: /* Driver requested deferred probing */ - dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name); + dev_err(dev, "Driver %s requests probe deferral\n", drv->name); + dev_warn_deferred(dev, "==== Driver %s requests probe deferral\n", drv->name); driver_deferred_probe_add(dev); /* Did a trigger occur while probing? Need to re-trigger if yes */ if (local_trigger_count != atomic_read(&deferred_trigger_count))