Re: svn commit: r324863 - in head/sys: kern sys
> On Oct 22, 2017, at 06: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 Thank you for fixing this. Question though — since u_char is unsigned, are you concerned about underflow (-1 -> 255)? Thanks, -Ngie signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r324863 - in head/sys: kern sys
On 10/22/17 19:37, Mateusz Guzik wrote:
On Sun, Oct 22, 2017 at 6:59 PM, Hans Petter Selasky
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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.hSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/sys/kdb.hSun 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:
8112873c D kdb_active
81128740 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
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r324863 - in head/sys: kern sys
On Sun, Oct 22, 2017 at 6:59 PM, Hans Petter Selasky
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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.hSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/sys/kdb.hSun 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:
8112873c D kdb_active
81128740 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
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r324863 - in head/sys: kern sys
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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.hSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/sys/kdb.hSun 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
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r324863 - in head/sys: kern sys
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.cSun Oct 22 12:12:52 2017(r324862)
> > +++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
> > @@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
> > #include
> > #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.hSun Oct 22 12:12:52 2017(r324862)
> > +++ head/sys/sys/kdb.hSun 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.
--
Mateusz Guzik
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r324863 - in head/sys: kern sys
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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???
--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r324863 - in head/sys: kern sys
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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(). */
Hi,
Sorry for nitpicking. I believe this variable is better suited as "int"?
Or am I wrong? What does the resulting assembly code look like?
--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r324863 - in head/sys: kern sys
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.cSun Oct 22 12:12:52 2017(r324862)
+++ head/sys/kern/subr_kdb.cSun Oct 22 13:42:56 2017(r324863)
@@ -50,7 +50,7 @@ __FBSDID("$FreeBSD$");
#include
#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(). */
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
