All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Byungchul Park <byungchul.park@lge.com>
Cc: peterz@infradead.org, tglx@linutronix.de,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	kernel-team@lge.com
Subject: Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default
Date: Wed, 18 Oct 2017 12:09:44 +0200	[thread overview]
Message-ID: <20171018100944.g2mc6yorhtm5piom@gmail.com> (raw)
In-Reply-To: <1508318006-2090-1-git-send-email-byungchul.park@lge.com>


* Byungchul Park <byungchul.park@lge.com> wrote:

> Johan Hovold reported a performance regression by crossrelease like:
> 
> > Boot time (from "Linux version" to login prompt) had in fact doubled
> > since 4.13 where it took 17 seconds (with my current config) compared to
> > the 35 seconds I now see with 4.14-rc4.
> >
> > I quick bisect pointed to lockdep and specifically the following commit:
> >
> > 	28a903f63ec0 ("locking/lockdep: Handle non(or multi)-acquisition
> > 	               of a crosslock")
> >
> > which I've verified is the commit which doubled the boot time (compared
> > to 28a903f63ec0^) (added by lockdep crossrelease series [1]).
> 
> Currently crossrelease performs unwind on every acquisition. But, that
> overloads systems too much. So this patch makes unwind optional and set
> it to N as default. Instead, it records only acquire_ip normally. Of
> course, unwind is sometimes required for full analysis. In that case, we
> can set CROSSRELEASE_STACK_TRACE to Y and use it.
> 
> In my qemu ubuntu machin (x86_64, 4 cores, 512M), the regression was
> fixed like, measuring timestamp of "Freeing unused kernel memory":
> 
> 1. No lockdep enabled
>    Average : 1.543353 secs
> 
> 2. Lockdep enabled
>    Average : 1.570806 secs
> 
> 3. Lockdep enabled + crossrelease enabled
>    Average : 1.870317 secs
> 
> 4. Lockdep enabled + crossrelease enabled + this patch applied
>    Average : 1.574143 secs

Ok, that looks really nice, recovers almost all of the lost performance, right?

Could you please run perf stat --null --repeat type of stats of a boot test (for 
example running init=/bin/true should boot up Qemu and make it exit), so that we 
can see how stable the numbers are and what the real slowdown is?

> +config CROSSRELEASE_STACK_TRACE
> +	bool "Record more than one entity of stack trace in crossrelease"
> +	depends on LOCKDEP_CROSSRELEASE
> +	default n
> +	help
> +	 Crossrelease feature needs to record stack traces for all
> +	 acquisitions for later use. And only acquire_ip is normally
> +	 recorded because the unwind operation is too expensive. However,
> +	 sometimes more than acquire_ip are required for full analysis.
> +	 In the case that we need to record more than one entity of
> +	 stack trace using unwind, this feature would be useful, with
> +	 taking more overhead.
> +
> +	 If unsure, say N.

Fixed the text for you:

> +	 The lockdep "cross-release" feature needs to record stack traces
> +	 (of calling functions) for all acquisitions, for eventual later use
> +	 during analysis.
> +	 By default only a single caller is recorded, because the unwind
> +	 operation can be very expensive with deeper stack chains.
> +	 However, sometimes deeper traces are required for full analysis.
> +	 This option turns on the saving of the full stack trace entries.
> +
> +	 If unsure, say N.

BTW., have you attempted limiting the depth of the stack traces? I suspect more 
than 2-4 are rarely required to disambiguate the calling context.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Byungchul Park <byungchul.park@lge.com>
Cc: peterz@infradead.org, tglx@linutronix.de,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	kernel-team@lge.com
Subject: Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default
Date: Wed, 18 Oct 2017 12:09:44 +0200	[thread overview]
Message-ID: <20171018100944.g2mc6yorhtm5piom@gmail.com> (raw)
In-Reply-To: <1508318006-2090-1-git-send-email-byungchul.park@lge.com>


* Byungchul Park <byungchul.park@lge.com> wrote:

> Johan Hovold reported a performance regression by crossrelease like:
> 
> > Boot time (from "Linux version" to login prompt) had in fact doubled
> > since 4.13 where it took 17 seconds (with my current config) compared to
> > the 35 seconds I now see with 4.14-rc4.
> >
> > I quick bisect pointed to lockdep and specifically the following commit:
> >
> > 	28a903f63ec0 ("locking/lockdep: Handle non(or multi)-acquisition
> > 	               of a crosslock")
> >
> > which I've verified is the commit which doubled the boot time (compared
> > to 28a903f63ec0^) (added by lockdep crossrelease series [1]).
> 
> Currently crossrelease performs unwind on every acquisition. But, that
> overloads systems too much. So this patch makes unwind optional and set
> it to N as default. Instead, it records only acquire_ip normally. Of
> course, unwind is sometimes required for full analysis. In that case, we
> can set CROSSRELEASE_STACK_TRACE to Y and use it.
> 
> In my qemu ubuntu machin (x86_64, 4 cores, 512M), the regression was
> fixed like, measuring timestamp of "Freeing unused kernel memory":
> 
> 1. No lockdep enabled
>    Average : 1.543353 secs
> 
> 2. Lockdep enabled
>    Average : 1.570806 secs
> 
> 3. Lockdep enabled + crossrelease enabled
>    Average : 1.870317 secs
> 
> 4. Lockdep enabled + crossrelease enabled + this patch applied
>    Average : 1.574143 secs

Ok, that looks really nice, recovers almost all of the lost performance, right?

Could you please run perf stat --null --repeat type of stats of a boot test (for 
example running init=/bin/true should boot up Qemu and make it exit), so that we 
can see how stable the numbers are and what the real slowdown is?

> +config CROSSRELEASE_STACK_TRACE
> +	bool "Record more than one entity of stack trace in crossrelease"
> +	depends on LOCKDEP_CROSSRELEASE
> +	default n
> +	help
> +	 Crossrelease feature needs to record stack traces for all
> +	 acquisitions for later use. And only acquire_ip is normally
> +	 recorded because the unwind operation is too expensive. However,
> +	 sometimes more than acquire_ip are required for full analysis.
> +	 In the case that we need to record more than one entity of
> +	 stack trace using unwind, this feature would be useful, with
> +	 taking more overhead.
> +
> +	 If unsure, say N.

Fixed the text for you:

> +	 The lockdep "cross-release" feature needs to record stack traces
> +	 (of calling functions) for all acquisitions, for eventual later use
> +	 during analysis.
> +	 By default only a single caller is recorded, because the unwind
> +	 operation can be very expensive with deeper stack chains.
> +	 However, sometimes deeper traces are required for full analysis.
> +	 This option turns on the saving of the full stack trace entries.
> +
> +	 If unsure, say N.

BTW., have you attempted limiting the depth of the stack traces? I suspect more 
than 2-4 are rarely required to disambiguate the calling context.

Thanks,

	Ingo

  parent reply	other threads:[~2017-10-18 10:09 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18  9:13 [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default Byungchul Park
2017-10-18  9:13 ` Byungchul Park
2017-10-18  9:13 ` [PATCH 2/2] lockdep: Remove BROKEN flag of LOCKDEP_CROSSRELEASE Byungchul Park
2017-10-18  9:13   ` Byungchul Park
2017-10-18 10:12   ` Ingo Molnar
2017-10-18 10:12     ` Ingo Molnar
2017-10-19  1:58     ` Byungchul Park
2017-10-19  1:58       ` Byungchul Park
2017-10-18 10:09 ` Ingo Molnar [this message]
2017-10-18 10:09   ` [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default Ingo Molnar
2017-10-19  4:32   ` Byungchul Park
2017-10-19  4:32     ` Byungchul Park
2017-10-19  5:57     ` Ingo Molnar
2017-10-19  5:57       ` Ingo Molnar
2017-10-19  6:11       ` Byungchul Park
2017-10-19  6:11         ` Byungchul Park
2017-10-19  6:22         ` Ingo Molnar
2017-10-19  6:22           ` Ingo Molnar
2017-10-19  6:36           ` Byungchul Park
2017-10-19  6:36             ` Byungchul Park
2017-10-19  8:05             ` Ingo Molnar
2017-10-19  8:05               ` Ingo Molnar
2017-10-19  6:22         ` Byungchul Park
2017-10-19  6:22           ` Byungchul Park
2017-10-19  8:10           ` Ingo Molnar
2017-10-19  8:10             ` Ingo Molnar
2017-10-19  9:02             ` 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)
2017-10-19  9:02               ` 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)
2017-10-19  9:41               ` Ingo Molnar
2017-10-19  9:41                 ` Ingo Molnar
2017-10-18 13:23 ` Thomas Gleixner
2017-10-18 13:23   ` Thomas Gleixner
2017-10-18 13:30   ` Ingo Molnar
2017-10-18 13:30     ` Ingo Molnar
2017-10-18 13:36     ` Thomas Gleixner
2017-10-18 13:36       ` Thomas Gleixner
2017-10-18 14:15       ` Matthew Wilcox
2017-10-18 14:15         ` Matthew Wilcox
2017-10-18 14:35         ` Thomas Gleixner
2017-10-18 14:35           ` Thomas Gleixner
2017-10-18 17:05           ` Ingo Molnar
2017-10-18 17:05             ` Ingo Molnar
2017-10-19  2:00       ` Byungchul Park
2017-10-19  2:00         ` Byungchul Park

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=20171018100944.g2mc6yorhtm5piom@gmail.com \
    --to=mingo@kernel.org \
    --cc=byungchul.park@lge.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.