Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1654817 > unrolled thread

[RFC PATCH 00/10] x86: undwarf unwinder

Started byJosh Poimboeuf <jpoimboe@redhat.com>
First post2017-06-01 07:50 +0200
Last post2017-06-02 12:50 +0200
Articles 20 on this page of 45 — 7 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH 00/10] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
      Re: [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Andy Lutomirski <luto@kernel.org> - 2017-06-01 16:10 +0200
        Re: [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 16:30 +0200
          Re: [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 16:30 +0200
            Re: [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Andy Lutomirski <luto@kernel.org> - 2017-06-01 16:50 +0200
              Re: [RFC PATCH 06/10] x86/entry: add CFI hint undwarf annotations Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 17:10 +0200
    [RFC PATCH 09/10] extable: add undwarf table sorting ability to sorttable script Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 08/10] extable: rename 'sortextable' script to 'sorttable' Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 05/10] objtool, x86: add facility for asm code to provide CFI hints Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
      Re: [RFC PATCH 05/10] objtool, x86: add facility for asm code to  provide CFI hints Andy Lutomirski <luto@kernel.org> - 2017-06-01 16:00 +0200
        Re: [RFC PATCH 05/10] objtool, x86: add facility for asm code to  provide CFI hints Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 16:20 +0200
          Re: [RFC PATCH 05/10] objtool, x86: add facility for asm code to  provide CFI hints Andy Lutomirski <luto@kernel.org> - 2017-06-01 16:50 +0200
            Re: [RFC PATCH 05/10] objtool, x86: add facility for asm code to  provide CFI hints Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 17:10 +0200
    [RFC PATCH 01/10] objtool: move checking code to check.c Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 07/10] x86/asm: add CFI hint annotations to sync_core() Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 04/10] objtool: add undwarf debuginfo generation Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
      Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 13:10 +0200
        Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 14:30 +0200
          Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-01 14:50 +0200
            Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 15:10 +0200
            Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 15:50 +0200
          Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 15:20 +0200
      Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 14:20 +0200
        Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 14:40 +0200
          Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 15:20 +0200
            Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 17:10 +0200
    [RFC PATCH 02/10] objtool, x86: add several functions and files to the objtool whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 07:50 +0200
    Re: [RFC PATCH 00/10] x86: undwarf unwinder Ingo Molnar <mingo@kernel.org> - 2017-06-01 08:10 +0200
      Re: [RFC PATCH 00/10] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 14:00 +0200
        Re: [RFC PATCH 00/10] x86: undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 14:20 +0200
          Re: [RFC PATCH 00/10] x86: undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-01 14:40 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-01 15:00 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 15:00 +0200
          Re: [RFC PATCH 00/10] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 14:50 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Peter Zijlstra <peterz@infradead.org> - 2017-06-01 15:30 +0200
              Re: [RFC PATCH 00/10] x86: undwarf unwinder Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-06-06 16:20 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Andy Lutomirski <luto@kernel.org> - 2017-06-01 16:00 +0200
        Re: [RFC PATCH 00/10] x86: undwarf unwinder Ingo Molnar <mingo@kernel.org> - 2017-06-01 16:00 +0200
          Re: [RFC PATCH 00/10] x86: undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-01 16:00 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-02 10:40 +0200
          Re: [RFC PATCH 00/10] x86: undwarf unwinder Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-01 16:10 +0200
          Re: [RFC PATCH 00/10] x86: undwarf unwinder Jiri Slaby <jslaby@suse.cz> - 2017-06-01 16:10 +0200
            Re: [RFC PATCH 00/10] x86: undwarf unwinder Mel Gorman <mgorman@suse.de> - 2017-06-02 12:50 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1655105 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromJiri Slaby <jslaby@suse.cz>
Date2017-06-01 14:50 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNxI5-2Aw-7@gated-at.bofh.it>
In reply to#1655083
On 06/01/2017, 02:26 PM, Josh Poimboeuf wrote:
> On Thu, Jun 01, 2017 at 01:05:39PM +0200, Peter Zijlstra wrote:
>> On Thu, Jun 01, 2017 at 12:44:16AM -0500, Josh Poimboeuf wrote:
>>
>>> +static struct undwarf *__undwarf_lookup(struct undwarf *undwarf,
>>> +					unsigned int num, unsigned long ip)
>>> +{
>>> +	struct undwarf *first = undwarf;
>>> +	struct undwarf *last = undwarf + num - 1;
>>> +	struct undwarf *mid;
>>> +	unsigned long u_ip;
>>> +
>>> +	while (first <= last) {
>>> +		mid = first + ((last - first) / 2);
>>> +		u_ip = undwarf_ip(mid);
>>> +
>>> +		if (ip >= u_ip) {
>>> +			if (ip < u_ip + mid->len)
>>> +				return mid;
>>> +			first = mid + 1;
>>> +		} else
>>> +			last = mid - 1;
>>> +	}
>>> +
>>> +	return NULL;
>>> +}
>>
>> That's a bog standard binary search thing, don't we have a helper for
>> that someplace?
> 
> I wasn't able to find one...

There is bsearch, but that doesn't support searching for a value in
between of 2 keys. I.e. what we typically have is these keys:
  some_function1 at 0x1000
  some_function2 at 0x2000
and we look for IP which can be e.g. 0x1010. The bsearch's cmp function
currently has no option to say, yes, the last one you asked me was the
right one, this one is after it already.

thanks,
-- 
js
suse labs

[toc] | [prev] | [next] | [standalone]


#1655121 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 15:10 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNy1r-2YH-7@gated-at.bofh.it>
In reply to#1655105
On Thu, Jun 01, 2017 at 02:47:48PM +0200, Jiri Slaby wrote:
> On 06/01/2017, 02:26 PM, Josh Poimboeuf wrote:
> > On Thu, Jun 01, 2017 at 01:05:39PM +0200, Peter Zijlstra wrote:
> >> On Thu, Jun 01, 2017 at 12:44:16AM -0500, Josh Poimboeuf wrote:
> >>
> >>> +static struct undwarf *__undwarf_lookup(struct undwarf *undwarf,
> >>> +					unsigned int num, unsigned long ip)
> >>> +{
> >>> +	struct undwarf *first = undwarf;
> >>> +	struct undwarf *last = undwarf + num - 1;
> >>> +	struct undwarf *mid;
> >>> +	unsigned long u_ip;
> >>> +
> >>> +	while (first <= last) {
> >>> +		mid = first + ((last - first) / 2);
> >>> +		u_ip = undwarf_ip(mid);
> >>> +
> >>> +		if (ip >= u_ip) {
> >>> +			if (ip < u_ip + mid->len)
> >>> +				return mid;
> >>> +			first = mid + 1;
> >>> +		} else
> >>> +			last = mid - 1;
> >>> +	}
> >>> +
> >>> +	return NULL;
> >>> +}
> >>
> >> That's a bog standard binary search thing, don't we have a helper for
> >> that someplace?
> > 
> > I wasn't able to find one...
> 
> There is bsearch, but that doesn't support searching for a value in
> between of 2 keys. I.e. what we typically have is these keys:
>   some_function1 at 0x1000
>   some_function2 at 0x2000
> and we look for IP which can be e.g. 0x1010. The bsearch's cmp function
> currently has no option to say, yes, the last one you asked me was the
> right one, this one is after it already.

Actually, I think bsearch would work with the latest version of the data
structure.  It now has a len field, so we can do a self-contained
compare.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1655153 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 15:50 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNyEa-3cj-7@gated-at.bofh.it>
In reply to#1655105
On Thu, Jun 01, 2017 at 02:47:48PM +0200, Jiri Slaby wrote:

> There is bsearch,

Shiny, should we move that into lib/sort.h and maybe get more people to
use it? search_extable() seems like something that could use it.

And __bug_table is something that seems to want all things sort applied.

[toc] | [prev] | [next] | [standalone]


#1655140 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 15:20 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNyb9-32h-39@gated-at.bofh.it>
In reply to#1655083
On Thu, Jun 01, 2017 at 07:26:51AM -0500, Josh Poimboeuf wrote:
> On Thu, Jun 01, 2017 at 01:05:39PM +0200, Peter Zijlstra wrote:
> > On Thu, Jun 01, 2017 at 12:44:16AM -0500, Josh Poimboeuf wrote:
> > 
> > > +static struct undwarf *__undwarf_lookup(struct undwarf *undwarf,
> > > +					unsigned int num, unsigned long ip)
> > > +{
> > > +	struct undwarf *first = undwarf;
> > > +	struct undwarf *last = undwarf + num - 1;
> > > +	struct undwarf *mid;
> > > +	unsigned long u_ip;
> > > +
> > > +	while (first <= last) {
> > > +		mid = first + ((last - first) / 2);
> > > +		u_ip = undwarf_ip(mid);
> > > +
> > > +		if (ip >= u_ip) {
> > > +			if (ip < u_ip + mid->len)
> > > +				return mid;
> > > +			first = mid + 1;
> > > +		} else
> > > +			last = mid - 1;
> > > +	}
> > > +
> > > +	return NULL;
> > > +}
> > 
> > That's a bog standard binary search thing, don't we have a helper for
> > that someplace?
> 
> I wasn't able to find one...

Yeah, I looked too and couldn't find it either. A well.

[toc] | [prev] | [next] | [standalone]


#1655077 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 14:20 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNxf3-2qB-7@gated-at.bofh.it>
In reply to#1654827
On Thu, Jun 01, 2017 at 12:44:16AM -0500, Josh Poimboeuf wrote:

> +static struct undwarf *undwarf_lookup(unsigned long ip)
> +{
> +	struct undwarf *undwarf;
> +	struct module *mod;
> +
> +	/* Look in vmlinux undwarf section: */
> +	undwarf = __undwarf_lookup(__undwarf_start, __undwarf_end - __undwarf_start, ip);
> +	if (undwarf)
> +		return undwarf;
> +
> +	/* Look in module undwarf sections: */
> +	preempt_disable();
> +	mod = __module_address(ip);
> +	if (!mod || !mod->arch.undwarf)
> +		goto module_out;
> +	undwarf = __undwarf_lookup(mod->arch.undwarf, mod->arch.num_undwarves, ip);
> +
> +module_out:
> +	preempt_enable();
> +	return undwarf;
> +}

> +bool unwind_next_frame(struct unwind_state *state)
> +{
> +	struct undwarf *undwarf;
> +	unsigned long cfa;
> +	bool indirect = false;
> +	enum stack_type prev_type = state->stack_info.type;
> +	unsigned long ip_p, prev_sp = state->sp;
> +
> +	if (unwind_done(state))
> +		return false;
> +
> +	/* Have we reached the end? */
> +	if (state->regs && user_mode(state->regs))
> +		goto done;
> +
> +	/* Look up the instruction address in the .undwarf table: */
> +	undwarf = undwarf_lookup(state->ip);
> +	if (!undwarf || undwarf->cfa_reg == UNDWARF_REG_UNDEFINED)
> +		goto done;
> +

	....

> +}
> +EXPORT_SYMBOL_GPL(unwind_next_frame);
> +
> +void __unwind_start(struct unwind_state *state, struct task_struct *task,
> +		    struct pt_regs *regs, unsigned long *first_frame)
> +{

	...

> +	while (!unwind_done(state) &&
> +	       (!on_stack(&state->stack_info, first_frame, sizeof(long)) ||
> +			state->sp <= (unsigned long)first_frame))
> +		unwind_next_frame(state);
> +}

So we do that lookup for every single frame. That's going to hurt.

Would it make sense to cache the last 'module' in an attempt to at least
avoid that lookup again? Something like so:

---
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -15,6 +15,7 @@ struct unwind_state {
 #if defined(CONFIG_UNDWARF_UNWINDER)
 	unsigned long sp, bp, ip;
 	struct pt_regs *regs;
+	struct module *mod;
 #elif defined(CONFIG_FRAME_POINTER)
 	bool got_irq;
 	unsigned long *bp, *orig_sp, ip;
--- a/arch/x86/kernel/unwind_undwarf.c
+++ b/arch/x86/kernel/unwind_undwarf.c
@@ -62,26 +62,45 @@ static struct undwarf *__undwarf_lookup(
 	return NULL;
 }
 
-static struct undwarf *undwarf_lookup(unsigned long ip)
+static struct undwarf *undwarf_lookup(struct unwind_state *state)
 {
+	struct module *mod = state->mod;
+	unsigned long ip = state->ip;
 	struct undwarf *undwarf;
-	struct module *mod;
+	unsigned int num;
 
-	/* Look in vmlinux undwarf section: */
-	undwarf = __undwarf_lookup(__undwarf_start, __undwarf_end - __undwarf_start, ip);
-	if (undwarf)
-		return undwarf;
+	if (mod) {
+		if (within_module(ip, mod)) {
+			undwarf = mod->arch.undwarf;
+			num	= mod->arch.num_undwarves;
+			goto lookup;
+		}
+		mod = NULL;
+	}
+
+	if (core_kernel_text(ip)) {
+		undwarf = __undwarf_start;
+		num	= __undwarf_end - __undwarf_start;
+		goto lookup;
+	}
 
-	/* Look in module undwarf sections: */
+	/*
+	 * Shut up the warning from __module_address(), regardless the undwarf
+	 * pointer can disappear from under us.
+	 */
 	preempt_disable();
 	mod = __module_address(ip);
+	preempt_enable();
+
 	if (!mod || !mod->arch.undwarf)
-		goto module_out;
-	undwarf = __undwarf_lookup(mod->arch.undwarf, mod->arch.num_undwarves, ip);
+		return NULL;
 
-module_out:
-	preempt_enable();
-	return undwarf;
+	undwarf	= mod->arch.undwarf;
+	num	= mod->arch.num_undwarves;
+
+lookup:
+	state->mod = mod;
+	return __undwarf_lookup(undwarf, num, ip);
 }
 
 static bool stack_access_ok(struct unwind_state *state, unsigned long addr,
@@ -168,7 +187,7 @@ bool unwind_next_frame(struct unwind_sta
 		goto done;
 
 	/* Look up the instruction address in the .undwarf table: */
-	undwarf = undwarf_lookup(state->ip);
+	undwarf = undwarf_lookup(state);
 	if (!undwarf || undwarf->cfa_reg == UNDWARF_REG_UNDEFINED)
 		goto done;
 

[toc] | [prev] | [next] | [standalone]


#1655097 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 14:40 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNxyq-2x0-17@gated-at.bofh.it>
In reply to#1655077
On Thu, Jun 01, 2017 at 02:13:56PM +0200, Peter Zijlstra wrote:
> So we do that lookup for every single frame. That's going to hurt.
> 
> Would it make sense to cache the last 'module' in an attempt to at least
> avoid that lookup again? Something like so:

The only thing with caching the module is, what if the module goes away?

Based on your previous comment I was thinking I would disable preemption
for the entire unwind_next_frame() step, but not *between* steps.  I
suppose we could require the unwind caller to disable preemption but I'd
like to avoid that if possible.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1655141 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 15:20 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNyb8-32h-33@gated-at.bofh.it>
In reply to#1655097
On Thu, Jun 01, 2017 at 07:36:09AM -0500, Josh Poimboeuf wrote:
> On Thu, Jun 01, 2017 at 02:13:56PM +0200, Peter Zijlstra wrote:
> > So we do that lookup for every single frame. That's going to hurt.
> > 
> > Would it make sense to cache the last 'module' in an attempt to at least
> > avoid that lookup again? Something like so:
> 
> The only thing with caching the module is, what if the module goes away?

Yeah.. *boom* ;-) We could of course play games with module_get() and
module_put(), but meh.

> Based on your previous comment I was thinking I would disable preemption
> for the entire unwind_next_frame() step, but not *between* steps.  I
> suppose we could require the unwind caller to disable preemption but I'd
> like to avoid that if possible.

Right, keeping it disabled across a frame should be ok I suppose.

[toc] | [prev] | [next] | [standalone]


#1655234 — Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 17:10 +0200
SubjectRe: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Message-ID<tNzTA-491-21@gated-at.bofh.it>
In reply to#1655141
On Thu, Jun 01, 2017 at 03:12:04PM +0200, Peter Zijlstra wrote:
> > Based on your previous comment I was thinking I would disable preemption
> > for the entire unwind_next_frame() step, but not *between* steps.  I
> > suppose we could require the unwind caller to disable preemption but I'd
> > like to avoid that if possible.
> 
> Right, keeping it disabled across a frame should be ok I suppose.

But then we'd either have to require the unwind user to explicitly
disable preemption, or we'd need to add a new unwind_end() interface
which the caller would be required to use when they're done unwinding.
Neither is ideal.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1654828 — [RFC PATCH 02/10] objtool, x86: add several functions and files to the objtool whitelist

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 07:50 +0200
Subject[RFC PATCH 02/10] objtool, x86: add several functions and files to the objtool whitelist
Message-ID<tNr9E-6Ua-27@gated-at.bofh.it>
In reply to#1654817
In preparation for an objtool rewrite which will have broader checks,
whitelist functions and files which cause problems because they do
unusual things with the stack.

These whitelists serve as a TODO list for which functions and files
don't yet have undwarf unwinder coverage.  Eventually most of the
whitelists can be removed in favor of manual CFI hint annotations or
objtool improvements.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/crypto/Makefile           | 2 ++
 arch/x86/crypto/sha1-mb/Makefile   | 2 ++
 arch/x86/crypto/sha256-mb/Makefile | 2 ++
 arch/x86/kernel/Makefile           | 1 +
 arch/x86/kernel/acpi/Makefile      | 2 ++
 arch/x86/kernel/kprobes/opt.c      | 9 ++++++++-
 arch/x86/kernel/reboot.c           | 2 ++
 arch/x86/kvm/svm.c                 | 2 ++
 arch/x86/kvm/vmx.c                 | 3 +++
 arch/x86/lib/msr-reg.S             | 8 ++++----
 arch/x86/net/Makefile              | 2 ++
 arch/x86/platform/efi/Makefile     | 1 +
 arch/x86/power/Makefile            | 2 ++
 arch/x86/xen/Makefile              | 3 +++
 kernel/kexec_core.c                | 4 +++-
 15 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 34b3fa2..9e32d40 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -2,6 +2,8 @@
 # Arch-specific CryptoAPI modules.
 #
 
+OBJECT_FILES_NON_STANDARD := y
+
 avx_supported := $(call as-instr,vpxor %xmm0$(comma)%xmm0$(comma)%xmm0,yes,no)
 avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
 				$(comma)4)$(comma)%ymm2,yes,no)
diff --git a/arch/x86/crypto/sha1-mb/Makefile b/arch/x86/crypto/sha1-mb/Makefile
index 2f87563..2e14acc 100644
--- a/arch/x86/crypto/sha1-mb/Makefile
+++ b/arch/x86/crypto/sha1-mb/Makefile
@@ -2,6 +2,8 @@
 # Arch-specific CryptoAPI modules.
 #
 
+OBJECT_FILES_NON_STANDARD := y
+
 avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
                                 $(comma)4)$(comma)%ymm2,yes,no)
 ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/crypto/sha256-mb/Makefile b/arch/x86/crypto/sha256-mb/Makefile
index 41089e7..45b4fca 100644
--- a/arch/x86/crypto/sha256-mb/Makefile
+++ b/arch/x86/crypto/sha256-mb/Makefile
@@ -2,6 +2,8 @@
 # Arch-specific CryptoAPI modules.
 #
 
+OBJECT_FILES_NON_STANDARD := y
+
 avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
                                 $(comma)4)$(comma)%ymm2,yes,no)
 ifeq ($(avx2_supported),yes)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 4b99423..3c7c419 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -29,6 +29,7 @@ OBJECT_FILES_NON_STANDARD_head_$(BITS).o		:= y
 OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o	:= y
 OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o		:= y
 OBJECT_FILES_NON_STANDARD_test_nx.o			:= y
+OBJECT_FILES_NON_STANDARD_paravirt_patch_$(BITS).o	:= y
 
 # If instrumentation of this dir is enabled, boot hangs during first second.
 # Probably could be more selective here, but note that files related to irqs,
diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 26b78d8..85a9e17 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_wakeup_$(BITS).o := y
+
 obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_$(BITS).o
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 901c640..69ea0bc 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -28,6 +28,7 @@
 #include <linux/kdebug.h>
 #include <linux/kallsyms.h>
 #include <linux/ftrace.h>
+#include <linux/frame.h>
 
 #include <asm/text-patching.h>
 #include <asm/cacheflush.h>
@@ -94,6 +95,7 @@ static void synthesize_set_arg1(kprobe_opcode_t *addr, unsigned long val)
 }
 
 asm (
+			"optprobe_template_func:\n"
 			".global optprobe_template_entry\n"
 			"optprobe_template_entry:\n"
 #ifdef CONFIG_X86_64
@@ -131,7 +133,12 @@ asm (
 			"	popf\n"
 #endif
 			".global optprobe_template_end\n"
-			"optprobe_template_end:\n");
+			"optprobe_template_end:\n"
+			".type optprobe_template_func, @function\n"
+			".size optprobe_template_func, .-optprobe_template_func\n");
+
+void optprobe_template_func(void);
+STACK_FRAME_NON_STANDARD(optprobe_template_func);
 
 #define TMPL_MOVE_IDX \
 	((long)&optprobe_template_val - (long)&optprobe_template_entry)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 2544700..67393fc 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -9,6 +9,7 @@
 #include <linux/sched.h>
 #include <linux/tboot.h>
 #include <linux/delay.h>
+#include <linux/frame.h>
 #include <acpi/reboot.h>
 #include <asm/io.h>
 #include <asm/apic.h>
@@ -123,6 +124,7 @@ void __noreturn machine_real_restart(unsigned int type)
 #ifdef CONFIG_APM_MODULE
 EXPORT_SYMBOL(machine_real_restart);
 #endif
+STACK_FRAME_NON_STANDARD(machine_real_restart);
 
 /*
  * Some Apple MacBook and MacBookPro's needs reboot=p to be able to reboot
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 183ddb2..bb7a502 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/amd-iommu.h>
 #include <linux/hashtable.h>
+#include <linux/frame.h>
 
 #include <asm/apic.h>
 #include <asm/perf_event.h>
@@ -4908,6 +4909,7 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu)
 
 	mark_all_clean(svm->vmcb);
 }
+STACK_FRAME_NON_STANDARD(svm_vcpu_run);
 
 static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root)
 {
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 72f7839..bc8b933 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/tboot.h>
 #include <linux/hrtimer.h>
+#include <linux/frame.h>
 #include "kvm_cache_regs.h"
 #include "x86.h"
 
@@ -8675,6 +8676,7 @@ static void vmx_handle_external_intr(struct kvm_vcpu *vcpu)
 			);
 	}
 }
+STACK_FRAME_NON_STANDARD(vmx_handle_external_intr);
 
 static bool vmx_has_high_real_mode_segbase(void)
 {
@@ -9051,6 +9053,7 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 	vmx_recover_nmi_blocking(vmx);
 	vmx_complete_interrupts(vmx);
 }
+STACK_FRAME_NON_STANDARD(vmx_vcpu_run);
 
 static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
 {
diff --git a/arch/x86/lib/msr-reg.S b/arch/x86/lib/msr-reg.S
index c815564..10ffa7e 100644
--- a/arch/x86/lib/msr-reg.S
+++ b/arch/x86/lib/msr-reg.S
@@ -13,14 +13,14 @@
 .macro op_safe_regs op
 ENTRY(\op\()_safe_regs)
 	pushq %rbx
-	pushq %rbp
+	pushq %r12
 	movq	%rdi, %r10	/* Save pointer */
 	xorl	%r11d, %r11d	/* Return value */
 	movl    (%rdi), %eax
 	movl    4(%rdi), %ecx
 	movl    8(%rdi), %edx
 	movl    12(%rdi), %ebx
-	movl    20(%rdi), %ebp
+	movl    20(%rdi), %r12d
 	movl    24(%rdi), %esi
 	movl    28(%rdi), %edi
 1:	\op
@@ -29,10 +29,10 @@ ENTRY(\op\()_safe_regs)
 	movl    %ecx, 4(%r10)
 	movl    %edx, 8(%r10)
 	movl    %ebx, 12(%r10)
-	movl    %ebp, 20(%r10)
+	movl    %r12d, 20(%r10)
 	movl    %esi, 24(%r10)
 	movl    %edi, 28(%r10)
-	popq %rbp
+	popq %r12
 	popq %rbx
 	ret
 3:
diff --git a/arch/x86/net/Makefile b/arch/x86/net/Makefile
index 90568c3..fefb4b6 100644
--- a/arch/x86/net/Makefile
+++ b/arch/x86/net/Makefile
@@ -1,4 +1,6 @@
 #
 # Arch-specific network modules
 #
+OBJECT_FILES_NON_STANDARD_bpf_jit.o += y
+
 obj-$(CONFIG_BPF_JIT) += bpf_jit.o bpf_jit_comp.o
diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile
index f1d83b3..2f56e1e 100644
--- a/arch/x86/platform/efi/Makefile
+++ b/arch/x86/platform/efi/Makefile
@@ -1,4 +1,5 @@
 OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
 
 obj-$(CONFIG_EFI) 		+= quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
 obj-$(CONFIG_EARLY_PRINTK_EFI)	+= early_printk.o
diff --git a/arch/x86/power/Makefile b/arch/x86/power/Makefile
index a6a198c..0504187 100644
--- a/arch/x86/power/Makefile
+++ b/arch/x86/power/Makefile
@@ -1,3 +1,5 @@
+OBJECT_FILES_NON_STANDARD_hibernate_asm_$(BITS).o := y
+
 # __restore_processor_state() restores %gs after S3 resume and so should not
 # itself be stack-protected
 nostackp := $(call cc-option, -fno-stack-protector)
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index fffb0a1..bced7a3 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -1,3 +1,6 @@
+OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_xen-pvh.o := y
+
 ifdef CONFIG_FUNCTION_TRACER
 # Do not profile debug and lowlevel utilities
 CFLAGS_REMOVE_spinlock.o = -pg
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index ae1a3ba..154ffb4 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -38,6 +38,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/compiler.h>
 #include <linux/hugetlb.h>
+#include <linux/frame.h>
 
 #include <asm/page.h>
 #include <asm/sections.h>
@@ -874,7 +875,7 @@ int kexec_load_disabled;
  * only when panic_cpu holds the current CPU number; this is the only CPU
  * which processes crash_kexec routines.
  */
-void __crash_kexec(struct pt_regs *regs)
+void __noclone __crash_kexec(struct pt_regs *regs)
 {
 	/* Take the kexec_mutex here to prevent sys_kexec_load
 	 * running on one cpu from replacing the crash kernel
@@ -896,6 +897,7 @@ void __crash_kexec(struct pt_regs *regs)
 		mutex_unlock(&kexec_mutex);
 	}
 }
+STACK_FRAME_NON_STANDARD(__crash_kexec);
 
 void crash_kexec(struct pt_regs *regs)
 {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1654834

FromIngo Molnar <mingo@kernel.org>
Date2017-06-01 08:10 +0200
Message-ID<tNrt0-7fK-17@gated-at.bofh.it>
In reply to#1654817
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> Here's the contents of the undwarf.txt file which explains the 'why' in
> more detail:

Ok, so the code quality looks pretty convincing to me - the new core 'undwarf' 
unwinder code is a _lot_ more readable than any of the Dwarf based attempts 
before.

That we control the debug info generation at build time is icing on the cake to 
me.

One thing I'd like to see on the list of benefits side of the equation is a size 
comparison of kernel .text, with frame pointers vs. undwarf, on 64-bit kernels.

Being able to generate more optimal code in the hottest code paths of the kernel 
is the _real_, primary upstream kernel benefit of a different debuginfo method - 
which has to be weighed against the pain of introducing a new unwinder. But this 
submission does not talk about that aspect at all, which should be fixed I think.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1655065

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 14:00 +0200
Message-ID<tNwVI-24Q-11@gated-at.bofh.it>
In reply to#1654834
On Thu, Jun 01, 2017 at 08:08:24AM +0200, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > Here's the contents of the undwarf.txt file which explains the 'why' in
> > more detail:
> 
> Ok, so the code quality looks pretty convincing to me - the new core 'undwarf' 
> unwinder code is a _lot_ more readable than any of the Dwarf based attempts 
> before.
> 
> That we control the debug info generation at build time is icing on the cake to 
> me.
> 
> One thing I'd like to see on the list of benefits side of the equation is a size 
> comparison of kernel .text, with frame pointers vs. undwarf, on 64-bit kernels.

Ok, will do a text size comparison.  The only difficulty I encountered
there is that the 'size' tool considers the .undwarf section to be text
for some reason.  So the "text" size grew considerably :-)

> Being able to generate more optimal code in the hottest code paths of the kernel 
> is the _real_, primary upstream kernel benefit of a different debuginfo method - 
> which has to be weighed against the pain of introducing a new unwinder. But this 
> submission does not talk about that aspect at all, which should be fixed I think.

Actually I devoted an entire one-sentence paragraph to performance in
the documentation:

  The simpler debuginfo format also enables the unwinder to be relatively
  fast, which is important for perf and lockdep.

But I'll try to highlight that a little more.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1655078

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 14:20 +0200
Message-ID<tNxf3-2qB-9@gated-at.bofh.it>
In reply to#1655065
On Thu, Jun 01, 2017 at 06:58:20AM -0500, Josh Poimboeuf wrote:
> > Being able to generate more optimal code in the hottest code paths of the kernel 
> > is the _real_, primary upstream kernel benefit of a different debuginfo method - 
> > which has to be weighed against the pain of introducing a new unwinder. But this 
> > submission does not talk about that aspect at all, which should be fixed I think.
> 
> Actually I devoted an entire one-sentence paragraph to performance in
> the documentation:
> 
>   The simpler debuginfo format also enables the unwinder to be relatively
>   fast, which is important for perf and lockdep.
> 
> But I'll try to highlight that a little more.

That's relative to a DWARF unwinder. It doesn't appear to be possible to
get anywhere near a frame-pointer unwinder due to having to do this
log(n) lookup for every single frame.

[toc] | [prev] | [next] | [standalone]


#1655103

FromJiri Slaby <jslaby@suse.cz>
Date2017-06-01 14:40 +0200
Message-ID<tNxyr-2x0-33@gated-at.bofh.it>
In reply to#1655078
On 06/01/2017, 02:17 PM, Peter Zijlstra wrote:
> On Thu, Jun 01, 2017 at 06:58:20AM -0500, Josh Poimboeuf wrote:
>>> Being able to generate more optimal code in the hottest code paths of the kernel 
>>> is the _real_, primary upstream kernel benefit of a different debuginfo method - 
>>> which has to be weighed against the pain of introducing a new unwinder. But this 
>>> submission does not talk about that aspect at all, which should be fixed I think.
>>
>> Actually I devoted an entire one-sentence paragraph to performance in
>> the documentation:
>>
>>   The simpler debuginfo format also enables the unwinder to be relatively
>>   fast, which is important for perf and lockdep.
>>
>> But I'll try to highlight that a little more.
> 
> That's relative to a DWARF unwinder. It doesn't appear to be possible to
> get anywhere near a frame-pointer unwinder due to having to do this
> log(n) lookup for every single frame.

This is ~ 20 times faster than my DWARF unwinder by a quick measurement
(20000 calls to save_stack_trace via single vfs_write).

perf profile, if you care:

__save_stack_trace
|
|--65.89%--unwind_next_frame
|          |
|          |--53.64%--__undwarf_lookup
|          |
|           --5.30%--deref_stack_reg
|                     |
|                      --2.32%--stack_access_ok
|
|--24.17%--__unwind_start
|          |
|          |--21.52%--unwind_next_frame
|          |          |
|          |          |--14.24%--__undwarf_lookup
|          |          |
|          |           --2.98%--deref_stack_reg
|          |                     |
|          |                      --1.32%--stack_access_ok
|          |
|           --1.32%--get_stack_info
|                     |
|                      --0.66%--in_task_stack
|
|--3.31%--unwind_get_return_address
|          __kernel_text_address
|          |
|          |--0.99%--is_ftrace_trampoline
|          |
|          |--0.99%--__is_insn_slot_addr
|          |          |
|          |           --0.66%--__rcu_read_unlock
|          |
|           --0.66%--is_bpf_text_address
|
 --1.66%--save_stack_address


-- 
js
suse labs

[toc] | [prev] | [next] | [standalone]


#1655115

FromJiri Slaby <jslaby@suse.cz>
Date2017-06-01 15:00 +0200
Message-ID<tNxRM-2EE-11@gated-at.bofh.it>
In reply to#1655103
On 06/01/2017, 02:52 PM, Josh Poimboeuf wrote:
>> This is ~ 20 times faster than my DWARF unwinder by a quick measurement
>> (20000 calls to save_stack_trace via single vfs_write).
> 
> Wow!

BTW the most time spent when unwinding DWARF was in those routines
decrypting uleb/sleb128 into u/longs.

thanks,
-- 
js
suse labs

[toc] | [prev] | [next] | [standalone]


#1655116

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 15:00 +0200
Message-ID<tNxRM-2EE-13@gated-at.bofh.it>
In reply to#1655103
On Thu, Jun 01, 2017 at 02:33:20PM +0200, Jiri Slaby wrote:
> On 06/01/2017, 02:17 PM, Peter Zijlstra wrote:
> > On Thu, Jun 01, 2017 at 06:58:20AM -0500, Josh Poimboeuf wrote:
> >>> Being able to generate more optimal code in the hottest code paths of the kernel 
> >>> is the _real_, primary upstream kernel benefit of a different debuginfo method - 
> >>> which has to be weighed against the pain of introducing a new unwinder. But this 
> >>> submission does not talk about that aspect at all, which should be fixed I think.
> >>
> >> Actually I devoted an entire one-sentence paragraph to performance in
> >> the documentation:
> >>
> >>   The simpler debuginfo format also enables the unwinder to be relatively
> >>   fast, which is important for perf and lockdep.
> >>
> >> But I'll try to highlight that a little more.
> > 
> > That's relative to a DWARF unwinder. It doesn't appear to be possible to
> > get anywhere near a frame-pointer unwinder due to having to do this
> > log(n) lookup for every single frame.
> 
> This is ~ 20 times faster than my DWARF unwinder by a quick measurement
> (20000 calls to save_stack_trace via single vfs_write).

Wow!  Thanks for quantifying that.  Looks like the lookup is indeed the
bottleneck as expected.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1655104

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-01 14:50 +0200
Message-ID<tNxI5-2Aw-5@gated-at.bofh.it>
In reply to#1655078
On Thu, Jun 01, 2017 at 02:17:21PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 01, 2017 at 06:58:20AM -0500, Josh Poimboeuf wrote:
> > > Being able to generate more optimal code in the hottest code paths of the kernel 
> > > is the _real_, primary upstream kernel benefit of a different debuginfo method - 
> > > which has to be weighed against the pain of introducing a new unwinder. But this 
> > > submission does not talk about that aspect at all, which should be fixed I think.
> > 
> > Actually I devoted an entire one-sentence paragraph to performance in
> > the documentation:
> > 
> >   The simpler debuginfo format also enables the unwinder to be relatively
> >   fast, which is important for perf and lockdep.
> > 
> > But I'll try to highlight that a little more.
> 
> That's relative to a DWARF unwinder.

Yes.

> It doesn't appear to be possible to get anywhere near a frame-pointer
> unwinder due to having to do this log(n) lookup for every single
> frame.

Hm, is there something faster, yet not substantially bigger?  Hash?
Trie?

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1655144

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-01 15:30 +0200
Message-ID<tNykO-35y-7@gated-at.bofh.it>
In reply to#1655104
On Thu, Jun 01, 2017 at 07:47:05AM -0500, Josh Poimboeuf wrote:

> > It doesn't appear to be possible to get anywhere near a frame-pointer
> > unwinder due to having to do this log(n) lookup for every single
> > frame.
> 
> Hm, is there something faster, yet not substantially bigger?  Hash?
> Trie?

Not sure how to make a Hash work with nearest neighbour searches. And a
trie will only give you a constant speedup over the binary search but
not an improvement in complexity IIRC.

[toc] | [prev] | [next] | [standalone]


#1658801

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-06-06 16:20 +0200
Message-ID<tPnuW-1tS-19@gated-at.bofh.it>
In reply to#1655144
On (06/01/17 15:25), Peter Zijlstra wrote:
[..]
> On Thu, Jun 01, 2017 at 07:47:05AM -0500, Josh Poimboeuf wrote:
> 
> > > It doesn't appear to be possible to get anywhere near a frame-pointer
> > > unwinder due to having to do this log(n) lookup for every single
> > > frame.
> > 
> > Hm, is there something faster, yet not substantially bigger?  Hash?
> > Trie?
> 
> Not sure how to make a Hash work with nearest neighbour searches. And a
> trie will only give you a constant speedup over the binary search but
> not an improvement in complexity IIRC.

by the way, as far as I know, there is *a bit* faster bsearch(). basically
there is a way to calculate pivot (middle element) using less instructions.

something like below, perhaps... may be can give some extra performance.
(not really tested. but I believe this is close to what gcc does in
libstdc++).


======

./scripts/bloat-o-meter lib/bsearch.o.old lib/bsearch.o.new
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24 (-24)
function                                     old     new   delta
bsearch                                      122      98     -24

---
 lib/bsearch.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/bsearch.c b/lib/bsearch.c
index e33c179089db..18b445b010c3 100644
--- a/lib/bsearch.c
+++ b/lib/bsearch.c
@@ -33,19 +33,21 @@
 void *bsearch(const void *key, const void *base, size_t num, size_t size,
 	      int (*cmp)(const void *key, const void *elt))
 {
-	size_t start = 0, end = num;
+	const char *pivot;
 	int result;
 
-	while (start < end) {
-		size_t mid = start + (end - start) / 2;
+	while (num > 0) {
+		pivot = base + (num >> 1) * size;
+		result = cmp(key, pivot);
 
-		result = cmp(key, base + mid * size);
-		if (result < 0)
-			end = mid;
-		else if (result > 0)
-			start = mid + 1;
-		else
-			return (void *)base + mid * size;
+		if (result == 0)
+			return (void *)pivot;
+
+		if (result > 0) {
+			base = pivot + size;
+			num--;
+		}
+		num >>= 1;
 	}
 
 	return NULL;
-- 
2.13.1

[toc] | [prev] | [next] | [standalone]


#1655159

FromAndy Lutomirski <luto@kernel.org>
Date2017-06-01 16:00 +0200
Message-ID<tNyNP-3fE-9@gated-at.bofh.it>
In reply to#1655104
On Thu, Jun 1, 2017 at 5:47 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Jun 01, 2017 at 02:17:21PM +0200, Peter Zijlstra wrote:
>> On Thu, Jun 01, 2017 at 06:58:20AM -0500, Josh Poimboeuf wrote:
>> > > Being able to generate more optimal code in the hottest code paths of the kernel
>> > > is the _real_, primary upstream kernel benefit of a different debuginfo method -
>> > > which has to be weighed against the pain of introducing a new unwinder. But this
>> > > submission does not talk about that aspect at all, which should be fixed I think.
>> >
>> > Actually I devoted an entire one-sentence paragraph to performance in
>> > the documentation:
>> >
>> >   The simpler debuginfo format also enables the unwinder to be relatively
>> >   fast, which is important for perf and lockdep.
>> >
>> > But I'll try to highlight that a little more.
>>
>> That's relative to a DWARF unwinder.
>
> Yes.
>
>> It doesn't appear to be possible to get anywhere near a frame-pointer
>> unwinder due to having to do this log(n) lookup for every single
>> frame.
>
> Hm, is there something faster, yet not substantially bigger?  Hash?
> Trie?

You have, roughly, a set of (key_start, value) pairs where, for any
given key, you want to find the (key_start, value) with the largest
key_start that doesn't exceed key.  Binary search gives you log_2(n)
queries, but its locality of reference sucks.  Here are two
suggestions for improving it:

1. Change the data layout.  Instead of having an array of undwarf
entries, have two parallel arrays, one with the ip addresses and one
with everything else.  This has no effect on the amount of space used,
but it makes the part used during search more compact.

2. Your key space is fairly small and your table entries should be
reasonably uniformly distributed.  Let the first IP you have unwind
data for be IP0.  Make an array mapping (IP - IP0) / B to the index of
the unwind entry for that IP for some suitable block size B.  Then, to
look up an IP, you'd find the indices of the unwind entries for (IP -
IP0) / B and (IP - IP0) / B + 1 and binary search between them.  With
constant B, this gives you O(1) performance instead of O(log(n)).
With B = 1, it's very fast, but the table is huge.  With B = 64k or
so, maybe you'd get a nice tradeoff of speedup vs size.  (With
modules, you'd presumably first search an rbtree to find which
instance of this data structure you're using and then do the lookup.)

3. Use a B-tree.  B-trees are simple if you don't need to deal with
insertion and deletion.  Presumably you'd choose your internal node
size so each internal node is exactly 64 or 128 bytes for good cache
performance.  This is still O(log(n)) and it uses more comparisons
than binary search, but you touch many fewer cache lines.

I expect that, if you do #1 and #2, you'd get excellent performance at
very little cost to the complexity of the code.

[toc] | [prev] | [next] | [standalone]


#1655160

FromIngo Molnar <mingo@kernel.org>
Date2017-06-01 16:00 +0200
Message-ID<tNyNP-3fE-11@gated-at.bofh.it>
In reply to#1655065
* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Thu, Jun 01, 2017 at 08:08:24AM +0200, Ingo Molnar wrote:
> > 
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > Here's the contents of the undwarf.txt file which explains the 'why' in
> > > more detail:
> > 
> > Ok, so the code quality looks pretty convincing to me - the new core 'undwarf' 
> > unwinder code is a _lot_ more readable than any of the Dwarf based attempts 
> > before.
> > 
> > That we control the debug info generation at build time is icing on the cake to 
> > me.
> > 
> > One thing I'd like to see on the list of benefits side of the equation is a size 
> > comparison of kernel .text, with frame pointers vs. undwarf, on 64-bit kernels.
> 
> Ok, will do a text size comparison.  The only difficulty I encountered
> there is that the 'size' tool considers the .undwarf section to be text
> for some reason.  So the "text" size grew considerably :-)

One trick I sometimes use is to only size some of the key builtin.o files.

> > Being able to generate more optimal code in the hottest code paths of the kernel 
> > is the _real_, primary upstream kernel benefit of a different debuginfo method - 
> > which has to be weighed against the pain of introducing a new unwinder. But this 
> > submission does not talk about that aspect at all, which should be fixed I think.
> 
> Actually I devoted an entire one-sentence paragraph to performance in
> the documentation:
> 
>   The simpler debuginfo format also enables the unwinder to be relatively
>   fast, which is important for perf and lockdep.
> 
> But I'll try to highlight that a little more.

That's not what I meant! The speedup comes from (hopefully) being able to disable 
CONFIG_FRAME_POINTER, which:

 - creates simpler/faster function prologues and epilogues - no managing of RBP 
   needed

 - gives one more generic purpose register to work from. This matters less on 
   64-bit kernels but it's a small effect.

I've seen numbers of 1-2% of instruction count reduction in common kernel 
workloads, which would be pretty significant on well cached workloads.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web