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

[Mateusz Guzik]

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.

-- 
Mateusz Guzik <mjguzik gmail.com>