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

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	live-patching@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>, Jiri Slaby <jslaby@suse.cz>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [RFC PATCH 10/10] x86/unwind: add undwarf unwinder
Date: Thu, 1 Jun 2017 07:26:51 -0500	[thread overview]
Message-ID: <20170601122651.g6xtofvyy3mbo7lq@treble> (raw)
In-Reply-To: <20170601110539.helelmmngwaba7fa@hirez.programming.kicks-ass.net>

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

> > +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;
> > +}
> 
> A few points here:
> 
>  - that first lookup is entirely pointless if !core_kernel_text(ip)

True.

>  - that preempt_{dis,en}able() muck is 'pointless', for while it shuts
>    up the warnings from __modules_address(), nothing preserves the
>    struct undwarf you get a pointer to after the preempt_enable().

Oops!

>  - what about 'interesting' things like, ftrace_trampoline, kprobe insn
>    slots and bpf text?

I think support for generated code can come later.  My current plan is
to have some kind of registration interface for associating debuginfo
with generated code.  Maybe as part of the generated code management
thing we talked about before:

  https://lkml.kernel.org/r/20170110085923.GD3092@twins.programming.kicks-ass.net

-- 
Josh