svn commit: r324863 - in head/sys: kern sys

[Hans Petter Selasky]

On 10/22/17 19:37, Mateusz Guzik wrote:
> On Sun, Oct 22, 2017 at 6:59 PM, Hans Petter Selasky <hps at selasky.org>
> wrote:
> 
>> On 10/22/17 16:50, Mateusz Guzik wrote:
>>
>>> On Sun, Oct 22, 2017 at 04:24:41PM +0200, Hans Petter Selasky wrote:
>>>
>>>> On 10/22/17 15:42, Mateusz Guzik wrote:
>>>>
>>>>> Author: mjg
>>>>> Date: Sun Oct 22 13:42:56 2017
>>>>> New Revision: 324863
>>>>> URL: https://svnweb.freebsd.org/changeset/base/324863
>>>>>
>>>>> Log:
>>>>>      Change kdb_active type to u_char.
>>>>>      Fixes warnings from gcc and keeps the small size. Perhaps nesting
>>>>>
>>>> should be moved
>>>
>>>>      to another variablle.
>>>>>      Reported by:    ngie
>>>>>
>>>>> Modified:
>>>>>      head/sys/kern/subr_kdb.c
>>>>>      head/sys/sys/kdb.h
>>>>>
>>>>> Modified: head/sys/kern/subr_kdb.c
>>>>>
>>>>> ============================================================
>>> ==================
>>>
>>>> --- head/sys/kern/subr_kdb.c    Sun Oct 22 12:12:52 2017    (r324862)
>>>>> +++ head/sys/kern/subr_kdb.c    Sun Oct 22 13:42:56 2017    (r324863)
>>>>> @@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
>>>>>     #include <machine/smp.h>
>>>>>     #endif
>>>>> -bool __read_frequently kdb_active = 0;
>>>>> +u_char __read_frequently kdb_active = 0;
>>>>>     static void *kdb_jmpbufp = NULL;
>>>>>     struct kdb_dbbe *kdb_dbbe = NULL;
>>>>>     static struct pcb kdb_pcb;
>>>>>
>>>>> Modified: head/sys/sys/kdb.h
>>>>>
>>>>> ============================================================
>>> ==================
>>>
>>>> --- head/sys/sys/kdb.h    Sun Oct 22 12:12:52 2017    (r324862)
>>>>> +++ head/sys/sys/kdb.h    Sun Oct 22 13:42:56 2017    (r324863)
>>>>> @@ -59,7 +59,7 @@ struct kdb_dbbe {
>>>>>         };                        \
>>>>>         DATA_SET(kdb_dbbe_set, name##_dbbe)
>>>>> -extern bool kdb_active;            /* Non-zero while in debugger. */
>>>>> +extern u_char kdb_active;        /* Non-zero while in debugger. */
>>>>>     extern int debugger_on_panic;        /* enter the debugger on panic.
>>>>>
>>>> */
>>>
>>>>     extern struct kdb_dbbe *kdb_dbbe;    /* Default debugger backend or
>>>>>
>>>> NULL. */
>>>
>>>>     extern struct trapframe *kdb_frame;    /* Frame to kdb_trap(). */
>>>>>
>>>>>
>>>>>
>>>> Should we add __aligned(8) to this definition?
>>>>
>>>> ./systm.h:#define    __read_frequently
>>>>
>>> __section(".data.read_frequently")
>>>
>>>>
>>>> It will prevent commonly read variables from residing in two different
>>>> cache-lines on x86 and amd64 at least???
>>>>
>>>>
>>> I don't follow. This would *increase* alignemnt requirement and in
>>> particular prevent bool variables from being put in consecutive bytes.
>>>
>>> To answer the question from your other e-mail, the bigger the type the
>>> worse it is as it takes more space. The idea is to change all frequently
>>> read and effectively bool variables from int to bool so that more of
>>> them fit in one cacheline.
>>>
>>> Right now there is nothing to nicely sort them to get rid of holes, but
>>> I'm tinkering with automagic size addition to section name.
>>>
>>>
>> Hi,
>>
>> The point is that for x86 there is no alignment so the variables get
>> packed back to back, and then you sometimes get not so smart layouts, like
>> that an integer crosses a cache line.
>>
>>
> They are aligned to their size and this creates holes, like here:
> 
> ffffffff8112873c D kdb_active
> ----------------
> ffffffff81128740 D audit_enabled
> 
> Regarding automation: Maybe the idea behind sysinit can be used:
>> sys/boot/usb/tools/sysinit.c
>>
> 
> I don't know how this can be plugged here. Would this require defining
> variables elsewhere?
> 
> Preferably they would be sorted by the linker.
> 

Hi,

If the linker supports this, that's the best. Else you'll need a custom 
tool. sysinit.c might give you some ideas how to implement it in a 
cross-OS way.

--HPS