From: Saravana Kannan <saravanak@google.com>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Tony Lindgren <tony@atomide.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Ulf Hansson <ulf.hansson@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
Peter Ujfalusi <peter.ujfalusi@ti.com>,
Tomi Valkeinen <tomi.valkeinen@ti.com>,
Linux-OMAP <linux-omap@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux PM <linux-pm@vger.kernel.org>,
Geert Uytterhoeven <geert+renesas@glider.be>
Subject: Re: Slow booting on x15
Date: Thu, 1 Oct 2020 11:24:12 -0700 [thread overview]
Message-ID: <CAGETcx-aiW251dhEXT1GNb9bi6YcX8W=jLBrro5CnPuEjGL09g@mail.gmail.com> (raw)
In-Reply-To: <60ea4b42-fbc2-2b80-7eab-8a236581d4c9@ti.com>
On Thu, Oct 1, 2020 at 5:56 AM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>
>
>
> On 01/10/2020 11:22, Laurent Pinchart wrote:
> > Hi Tony,
> >
> > On Thu, Oct 01, 2020 at 11:17:48AM +0300, Tony Lindgren wrote:
> >> * Tony Lindgren <tony@atomide.com> [201001 07:53]:
> >>> * Peter Ujfalusi <peter.ujfalusi@ti.com> [200930 12:41]:
> >>>> Fwiw on my beagle x15
> >>>>
> >>>> v5.8
> >>>> [ 9.908787] Run /sbin/init as init process
> >>>>
> >>>> v5.9-rc7
> >>>> [ 15.085373] Run /sbin/init as init process
> >>>>
> >>>>
> >>>> It appears to be 'fixed' in next-20200928: the board does not even boot.
> >>>
> >>> Yeah so it seems :(
> >>>
> >>>> next-20200928 on omap5
> >>>> [ 9.936806] Run /sbin/init as init process
> >>>>
> >>>>
> >>>> -rc7 spends most of it's time:
> >>>> [ 7.635530] Micrel KSZ9031 Gigabit PHY 48485000.mdio:01: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=48485000.mdio:01, irq=POLL)
> >>>> [ 14.956671] cpsw 48484000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> >>>> [ 15.005211] IP-Config: Complete:
> >>>
> >>> Booting with initcall_debug I see this with current Linux next:
> >>>
> >>> ...
> >>> [ 1.697313] cpuidle: using governor menu
> >>> [ 1.701353] initcall init_menu+0x0/0xc returned 0 after 0 usecs
> >>> [ 1.707458] calling gpmc_init+0x0/0x10 @ 1
> >>> [ 1.711784] initcall gpmc_init+0x0/0x10 returned 0 after 0 usecs
> >>> [ 1.717974] calling omap3_l3_init+0x0/0x10 @ 1
> >>> [ 1.722653] initcall omap3_l3_init+0x0/0x10 returned 0 after 0 usecs
> >>> [ 1.729201] calling omap_l3_init+0x0/0x10 @ 1
> >>> [ 1.733791] initcall omap_l3_init+0x0/0x10 returned 0 after 0 usecs
> >>> [ 1.740314] calling gate_vma_init+0x0/0x70 @ 1
> >>> [ 1.744976] initcall gate_vma_init+0x0/0x70 returned 0 after 0 usecs
> >>> [ 1.751522] calling customize_machine+0x0/0x30 @ 1
> >>> [ 3.823114] initcall customize_machine+0x0/0x30 returned 0 after 2011718 usecs
> >>> [ 3.830566] calling init_atags_procfs+0x0/0xec @ 1
> >>> [ 3.835583] No ATAGs?
> >>
> >> And the long time above with customize_machine() ends up being
> >> pdata_quirks_init() calling of_platform_populate().
> >
> > That's what the delay is for me (I think I've reported that initially).
> >
> >>> Laurent & Tomi, care to check what you guys see in the slow booting case
> >>> after booting with initcall_debug?
> >>
> >> But maybe the long delay is something else for you guys so please check.
> >
>
> It's all devlink :( Looks like sometimes, improvements (PM) could became so complicated
> that time required to execute such algorithms may completely eliminate all expected benefits.
> Will not be surprised if PM consumption also increased instead of decreasing in some cases.
>
> not sure if it's 100% correct, but below diff reduces boot time
> from 7.6sec to 3.7sec :P
>
> before:
> [ 0.053870] cpuidle: using governor menu
> [ 2.505971] No ATAGs?
> ...
> [ 7.562317] Freeing unused kernel memory: 1024K
>
> after:
> [ 0.053800] cpuidle: using governor menu
> [ 0.136853] No ATAGs?
> [ 3.716218] devtmpfs: mounted
> [ 3.719628] Freeing unused kernel memory: 1024K
> [ 3.724266] Run /sbin/init as init process
>
> ----
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 071f04da32c8..e0cc37ed46ca 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -481,6 +481,7 @@ int of_platform_populate(struct device_node *root,
> pr_debug(" starting at: %pOF\n", root);
>
> device_links_supplier_sync_state_pause();
> + fw_devlink_pause();
> for_each_child_of_node(root, child) {
> rc = of_platform_bus_create(child, matches, lookup, parent, true);
> if (rc) {
> @@ -488,6 +489,7 @@ int of_platform_populate(struct device_node *root,
> break;
> }
> }
> + fw_devlink_resume();
> device_links_supplier_sync_state_resume();
>
> of_node_set_flag(root, OF_POPULATED_BUS);
> @@ -538,9 +540,7 @@ static int __init of_platform_default_populate_init(void)
> }
>
> /* Populate everything else. */
> - fw_devlink_pause();
> of_platform_default_populate(NULL, NULL, NULL);
> - fw_devlink_resume();
Your analysis is right, but this change is not safe. You'll get an
unlocked linked list trampling if you call it outside of where it's
called now. That's explicitly why I didn't do it the way this patch
does it.
To explain more, if you call fw_devlink_pause/resume() inside
of_platform_populate() you can end up calling it in the context of
another device's probe function. When a device's probe function is
called, a has a bunch of other locks held and you'll cause a deadlock.
To avoid that, I had to use defer_fw_devlink_lock to manage the list
used by fw_devlink_pause/resume().
I'll add more details later. But yeah, this patch isn't safe as is.
-Saravana
next prev parent reply other threads:[~2020-10-01 18:24 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 15:25 Slow booting on x15 Tomi Valkeinen
2020-09-18 15:58 ` Laurent Pinchart
2020-09-23 7:07 ` Tony Lindgren
2020-09-23 11:13 ` Laurent Pinchart
2020-09-24 5:42 ` Tony Lindgren
2020-09-24 5:53 ` Tony Lindgren
2020-09-24 6:04 ` Tomi Valkeinen
2020-09-24 6:08 ` Tony Lindgren
2020-09-24 13:30 ` Laurent Pinchart
2020-09-25 11:51 ` Tony Lindgren
2020-09-25 11:58 ` Laurent Pinchart
2020-09-30 5:20 ` Tony Lindgren
2020-09-30 12:41 ` Peter Ujfalusi
2020-10-01 7:53 ` Tony Lindgren
2020-10-01 8:17 ` Tony Lindgren
2020-10-01 8:22 ` Laurent Pinchart
2020-10-01 12:56 ` Grygorii Strashko
2020-10-01 13:11 ` Geert Uytterhoeven
2020-10-01 13:49 ` Grygorii Strashko
2020-10-01 18:24 ` Saravana Kannan [this message]
2020-10-01 19:43 ` Grygorii Strashko
2020-10-01 22:22 ` Saravana Kannan
2020-10-01 22:30 ` Saravana Kannan
2020-10-01 22:38 ` Laurent Pinchart
2020-10-01 22:44 ` Saravana Kannan
2020-10-01 22:59 ` [PATCH v1] of: platform: Batch fwnode parsing in the init_machine() path Saravana Kannan
2020-10-01 23:19 ` Laurent Pinchart
2020-10-02 11:40 ` Grygorii Strashko
2020-10-02 15:03 ` Grygorii Strashko
2020-10-02 17:48 ` Saravana Kannan
2020-10-02 18:11 ` Grygorii Strashko
2020-10-02 14:07 ` Rob Herring
2020-10-02 17:51 ` Saravana Kannan
2020-10-02 17:54 ` Laurent Pinchart
2020-10-02 17:58 ` Saravana Kannan
2020-10-02 18:27 ` Laurent Pinchart
2020-10-02 18:35 ` Grygorii Strashko
2020-10-02 19:56 ` Saravana Kannan
2020-10-03 0:13 ` Laurent Pinchart
2020-10-27 3:29 ` Saravana Kannan
2020-10-28 7:34 ` Tomi Valkeinen
2020-10-02 20:29 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGETcx-aiW251dhEXT1GNb9bi6YcX8W=jLBrro5CnPuEjGL09g@mail.gmail.com' \
--to=saravanak@google.com \
--cc=geert+renesas@glider.be \
--cc=gregkh@linuxfoundation.org \
--cc=grygorii.strashko@ti.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peter.ujfalusi@ti.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=tomi.valkeinen@ti.com \
--cc=tony@atomide.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).