Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 22 Oct 2017 18:59:19 +0200
From:      Hans Petter Selasky <hps@selasky.org>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        Mateusz Guzik <mjg@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r324863 - in head/sys: kern sys
Message-ID:  <b4cf9de2-cafc-1502-7f63-50a04d68cafe@selasky.org>
In-Reply-To: <CAGudoHFcBmsncg5k2cc4CGCbX6mhNOmcFKTNjWMgWskVbYWzNQ@mail.gmail.com>
References:  <201710221342.v9MDguCC074682@repo.freebsd.org> <316ecd86-a508-fb36-e33c-ba32f5cb8073@selasky.org> <CAGudoHFcBmsncg5k2cc4CGCbX6mhNOmcFKTNjWMgWskVbYWzNQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

Regarding automation: Maybe the idea behind sysinit can be used:
sys/boot/usb/tools/sysinit.c

--HPS



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b4cf9de2-cafc-1502-7f63-50a04d68cafe>