[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrXhP9wvFjj_krbzUZB5Zpx0cXc55KV6iQhdWEGf3LPQWA@mail.gmail.com>
Date: Mon, 8 May 2017 19:31:50 -0700
From: Andy Lutomirski <luto@...nel.org>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Andy Lutomirski <luto@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Jiri Slaby <jslaby@...e.cz>,
Andrew Morton <akpm@...ux-foundation.org>,
live-patching@...r.kernel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
"the arch/x86 maintainers" <x86@...nel.org>,
Jiri Kosina <jikos@...nel.org>
Subject: Re: [PATCH 7/7] DWARF: add the config option
On Mon, May 8, 2017 at 6:38 PM, Josh Poimboeuf <jpoimboe@...hat.com> 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@...hat.com> 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
Powered by blists - more mailing lists