linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Saravana Kannan <saravanak@google.com>
Cc: Grygorii Strashko <grygorii.strashko@ti.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: Fri, 2 Oct 2020 01:38:48 +0300	[thread overview]
Message-ID: <20201001223848.GE3722@pendragon.ideasonboard.com> (raw)
In-Reply-To: <CAGETcx_gCJuNqhBo+a4f0YHeUQJm+WitREw=hvu_5VBMZENCuw@mail.gmail.com>

Hi Saravana,

On Thu, Oct 01, 2020 at 03:30:39PM -0700, Saravana Kannan wrote:
> On Thu, Oct 1, 2020 at 3:22 PM Saravana Kannan wrote:
> > On Thu, Oct 1, 2020 at 12:43 PM Grygorii Strashko wrote:
> > > On 01/10/2020 21:24, Saravana Kannan wrote:
> > > > On Thu, Oct 1, 2020 at 5:56 AM Grygorii Strashko wrote:
> > > >> On 01/10/2020 11:22, Laurent Pinchart wrote:
> > > >>> 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.
> > >
> > > I assume you going to fix it your way, right?
> >
> > I'll be glad to fix this if I find a way out. But currently, there's
> > no "my way" for a generic fix to use inside of_platform_populate().
> > However...
> >
> > > Just FYI. On ARM main DT population path is:
> > >
> > > arch_initcall(customize_machine)
> > >         machine_desc->init_machine();
> > >                 of_platform_default_populate();
> > >                 -- or --
> > >                 of_platform_populate();
> > >
> > > the of_platform_default_populate_init() called later due to make/linker dependencies.
> > >
> > > As result, existing pause/resume optimizations don't fully working.
> >
> > We can just add this optimization around all the other places the top
> > level addition of DT devices is done. I wasn't aware of (more like
> > forgot) the init_machine() path.
> >
> > Can you give the call path in your case? Just to make it a bit easier for me.
> 
> I think I have a simple solution. I can just move the
> fw_devlink_pause/resume() inside of_platform_default_populate() and
> call those only if "root" == NULL.

I'd be happy to test a patch. On my device, the initial
of_platform_populate() call from machine_desc->init_machine() takes
between 6 and 10 seconds to run with v5.9-rc5, compared to 200ms on
v5.7. That's a fairly bad regression, all the people who have worked
hard so reduce boot time would really hate this :-)

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2020-10-01 22:39 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
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 [this message]
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=20201001223848.GE3722@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=grygorii.strashko@ti.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=saravanak@google.com \
    --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).