Re: [PATCH 7/7] DWARF: add the config option

From: Andy Lutomirski
Date: Mon May 08 2017 - 22:32:20 EST


On Mon, May 8, 2017 at 6:38 PM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> On Mon, May 08, 2017 at 05:21:24PM -0700, Andy Lutomirski wrote:
>> On Sun, May 7, 2017 at 9:55 AM, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>> > struct undwarf {
>> > unsigned int ip; /* instruction pointer (relative offset from base) */
>> > unsigned prev_frame:13; /* offset to previous frame from current stack pointer */
>> > unsigned regs:1; /* whether prev_frame contains entry regs (regs->ip) */
>> > unsigned align:2; /* some details for dealing with gcc stack realignment */
>> > } __packed;
>> >
>> > extern struct undwarf undwarves[];
>>
>> Some comments in case you're actually planning to do this:
>>
>> 'unsigned int ip' is the majority of the size of this thing. It might
>> be worth trying to store a lot fewer bits. You could split the
>> structure into:
>>
>> struct undwarf_header {
>> unsigned long start_ip;
>> unsigned align:2; /* i'm assuming this rarely changes */
>> ...;
>> unsigned int offset_to_details;
>> };
>>
>> and
>>
>> struct undwarf_details {
>> unsigned short ip_offset;
>> unsigned short prev_frame;
>> };
>>
>> and you'd find the details by first looking up the last header before
>> the ip and then finding the details starting at (uintptr_t)header +
>> header->offset_to_details.
>
> Good idea. According to some back-of-a-napkin math, a scheme like this
> could reduce the data size from 1.8M down to 1.2M with my kernel config,
> a not-too-shabby 600k savings.
>
>> Also, don't you need some indication of which reg is the base from
>> which you find previous frame? After all, sometimes GCC will emit a
>> frame pointer even in an otherwise frame-pointer-omitting kernel.
>
> I don't think we *need* to do that. I believe the base reg can just
> always[*] be the stack pointer, even with frame pointers.

What if there are functions that use alloca or variable length arrays
on the stack? Functions using AHASH_REQUEST_ON_STACK come to mind.

--Andy