linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Waiman Long <longman@redhat.com>, Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will.deacon@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	linux-mm@kvack.org, Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Tejun Heo <tj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC PATCH 00/12] locking/lockdep: Add a new class of terminal locks
Date: Fri, 9 Nov 2018 09:04:12 +0100	[thread overview]
Message-ID: <20181109080412.GC86700@gmail.com> (raw)
In-Reply-To: <1541709268-3766-1-git-send-email-longman@redhat.com>


* Waiman Long <longman@redhat.com> wrote:

> The purpose of this patchset is to add a new class of locks called
> terminal locks and converts some of the low level raw or regular
> spinlocks to terminal locks. A terminal lock does not have forward
> dependency and it won't allow a lock or unlock operation on another
> lock. Two level nesting of terminal locks is allowed, though.
> 
> Only spinlocks that are acquired with the _irq/_irqsave variants or
> acquired in an IRQ disabled context should be classified as terminal
> locks.
> 
> Because of the restrictions on terminal locks, we can do simple checks on
> them without using the lockdep lock validation machinery. The advantages
> of making these changes are as follows:
> 
>  1) The lockdep check will be faster for terminal locks without using
>     the lock validation code.
>  2) It saves table entries used by the validation code and hence make
>     it harder to overflow those tables.
> 
> In fact, it is possible to overflow some of the tables by running
> a variety of different workloads on a debug kernel. I have seen bug
> reports about exhausting MAX_LOCKDEP_KEYS, MAX_LOCKDEP_ENTRIES and
> MAX_STACK_TRACE_ENTRIES. This patch will help to reduce the chance
> of overflowing some of the tables.
> 
> Performance wise, there was no statistically significant difference in
> performanace when doing a parallel kernel build on a debug kernel.

Could you please measure a locking intense workload instead, such as:

   $ perf stat --null --sync --repeat 10 perf bench sched messaging

and profile which locks used there could be marked terminal, and measure 
the before/after performance impact?

> Below were selected output lines from the lockdep_stats files of the
> patched and unpatched kernels after bootup and running parallel kernel
> builds.
> 
>   Item                     Unpatched kernel  Patched kernel  % Change
>   ----                     ----------------  --------------  --------
>   direct dependencies           9732             8994          -7.6%
>   dependency chains            18776            17033          -9.3%
>   dependency chain hlocks      76044            68419         -10.0%
>   stack-trace entries         110403           104341          -5.5%

That's pretty impressive!

> There were some reductions in the size of the lockdep tables. They were
> not significant, but it is still a good start to rein in the number of
> entries in those tables to make it harder to overflow them.

Agreed.

BTW., if you are interested in more radical approaches to optimize 
lockdep, we could also add a static checker via objtool driven call graph 
analysis, and mark those locks terminal that we can prove are terminal.

This would require the unified call graph of the kernel image and of all 
modules to be examined in a final pass, but that's within the principal 
scope of objtool. (This 'final pass' could also be done during bootup, at 
least in initial versions.)

Note that beyond marking it 'terminal' such a static analysis pass would 
also allow the detection of obvious locking bugs at the build (or boot) 
stage already - plus it would allow the disabling of lockdep for 
self-contained locks that don't interact with anything else.

I.e. the static analysis pass would 'augment' lockdep and leave only 
those locks active for runtime lockdep tracking whose dependencies it 
cannot prove to be correct yet.

Thanks,

	Ingo

  parent reply	other threads:[~2018-11-09  8:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08 20:34 Waiman Long
2018-11-08 20:34 ` [RFC PATCH 01/12] locking/lockdep: Rework lockdep_set_novalidate_class() Waiman Long
2018-11-10 14:14   ` Peter Zijlstra
2018-11-11  0:26     ` Waiman Long
2018-11-11  1:28       ` Peter Zijlstra
2018-11-08 20:34 ` [RFC PATCH 02/12] locking/lockdep: Add a new terminal lock type Waiman Long
2018-11-10 14:17   ` Peter Zijlstra
2018-11-11  0:28     ` Waiman Long
2018-11-08 20:34 ` [RFC PATCH 03/12] locking/lockdep: Add DEFINE_TERMINAL_SPINLOCK() and related macros Waiman Long
2018-11-08 20:34 ` [RFC PATCH 04/12] printk: Make logbuf_lock a terminal lock Waiman Long
2018-11-08 20:34 ` [RFC PATCH 05/12] debugobjects: Mark pool_lock as " Waiman Long
2018-11-08 20:34 ` [RFC PATCH 06/12] debugobjects: Move printk out of db lock critical sections Waiman Long
2018-11-08 20:34 ` [RFC PATCH 07/12] locking/lockdep: Add support for nested terminal locks Waiman Long
2018-11-10 14:20   ` Peter Zijlstra
2018-11-11  0:30     ` Waiman Long
2018-11-11  1:30       ` Peter Zijlstra
2018-11-08 20:34 ` [RFC PATCH 08/12] debugobjects: Make object hash locks " Waiman Long
2018-11-08 20:34 ` [RFC PATCH 09/12] lib/stackdepot: Make depot_lock a terminal spinlock Waiman Long
2018-11-08 20:34 ` [RFC PATCH 10/12] locking/rwsem: Mark rwsem.wait_lock as a terminal lock Waiman Long
2018-11-08 20:34 ` [RFC PATCH 11/12] cgroup: Mark the rstat percpu lock as terminal Waiman Long
2018-11-08 20:34 ` [RFC PATCH 12/12] mm/kasan: Make quarantine_lock a terminal lock Waiman Long
2018-11-09  8:04 ` Ingo Molnar [this message]
2018-11-09 15:48   ` [RFC PATCH 00/12] locking/lockdep: Add a new class of terminal locks Waiman Long
2018-11-12  5:15     ` Ingo Molnar
2018-11-10 14:10   ` Peter Zijlstra
2018-11-10 23:35     ` Waiman Long
2018-11-12  5:10       ` Ingo Molnar
2018-11-12  5:53         ` Josh Poimboeuf
2018-11-12  6:30           ` Ingo Molnar
2018-11-12 22:22             ` Josh Poimboeuf
2018-11-12 22:56               ` Waiman Long

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=20181109080412.GC86700@gmail.com \
    --to=mingo@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=jpoimboe@redhat.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --subject='Re: [RFC PATCH 00/12] locking/lockdep: Add a new class of terminal locks' \
    /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

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).