Re: svn commit: r335879 - in head/sys: conf kern sys
On Sun, Jul 8, 2018 at 7:22 AM, Mark Johnston wrote:
> On Tue, Jul 03, 2018 at 01:55:10AM +, Matt Macy wrote:
>> Author: mmacy
>> Date: Tue Jul 3 01:55:09 2018
>> New Revision: 335879
>> URL: https://svnweb.freebsd.org/changeset/base/335879
>>
>> Log:
>> make critical_{enter, exit} inline
>>
>> Avoid pulling in all of the dependencies by
>> automatically generating a stripped down thread_lite exporting
>> only the fields of interest. The field declarations are type checked
>> against the original and the offsets of the generated result is
>> automatically checked.
>>
>> kib has expressed disagreement and would have preferred to simply
>> use genassym style offsets (which loses type check enforcement).
>> jhb has expressed dislike of it due to header pollution and a
>> duplicate structure. He would have preferred to just have defined
>> thread in _thread.h. Nonetheless, he admits that this is the only
>> viable solution at the moment.
>>
>> The impetus for this came from mjg's D15331:
>> "Inline critical_enter/exit for amd64"
>>
>> Reviewed by: jeff
>> Differential Revision: https://reviews.freebsd.org/D16078
>>
>> [...]
>> +#if defined(KLD_MODULE) || defined(KTR_CRITICAL) || !defined(_KERNEL) ||
>> defined(GENOFFSET)
>> +#define critical_enter() critical_enter_KBI()
>> +#define critical_exit() critical_exit_KBI()
>> +#else
>> +static __inline void
>> +critical_enter(void)
>> +{
>> + struct thread_lite *td;
>> +
>> + td = (struct thread_lite *)curthread;
>> + td->td_critnest++;
>
> Don't we need a compiler barrier here?
>
We definitely do. Not sure how that got lost :( Will fix.
-M
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r335879 - in head/sys: conf kern sys
On Tue, Jul 03, 2018 at 01:55:10AM +, Matt Macy wrote:
> Author: mmacy
> Date: Tue Jul 3 01:55:09 2018
> New Revision: 335879
> URL: https://svnweb.freebsd.org/changeset/base/335879
>
> Log:
> make critical_{enter, exit} inline
>
> Avoid pulling in all of the dependencies by
> automatically generating a stripped down thread_lite exporting
> only the fields of interest. The field declarations are type checked
> against the original and the offsets of the generated result is
> automatically checked.
>
> kib has expressed disagreement and would have preferred to simply
> use genassym style offsets (which loses type check enforcement).
> jhb has expressed dislike of it due to header pollution and a
> duplicate structure. He would have preferred to just have defined
> thread in _thread.h. Nonetheless, he admits that this is the only
> viable solution at the moment.
>
> The impetus for this came from mjg's D15331:
> "Inline critical_enter/exit for amd64"
>
> Reviewed by: jeff
> Differential Revision: https://reviews.freebsd.org/D16078
>
> [...]
> +#if defined(KLD_MODULE) || defined(KTR_CRITICAL) || !defined(_KERNEL) ||
> defined(GENOFFSET)
> +#define critical_enter() critical_enter_KBI()
> +#define critical_exit() critical_exit_KBI()
> +#else
> +static __inline void
> +critical_enter(void)
> +{
> + struct thread_lite *td;
> +
> + td = (struct thread_lite *)curthread;
> + td->td_critnest++;
Don't we need a compiler barrier here?
> +}
> +
> +static __inline void
> +critical_exit(void)
> +{
> + struct thread_lite *td;
> +
> + td = (struct thread_lite *)curthread;
> + KASSERT(td->td_critnest != 0,
> + ("critical_exit: td_critnest == 0"));
> + td->td_critnest--;
> + __compiler_membar();
> + if (__predict_false(td->td_owepreempt))
> + critical_exit_preempt();
> +
> +}
> +#endif
> +
> +
> #ifdef EARLY_PRINTF
> typedef void early_putc_t(int ch);
> extern early_putc_t *early_putc;
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r335879 - in head/sys: conf kern sys
genoffset_test tests that the offsets match up
On Tue, Jul 3, 2018 at 11:02 AM, Bryan Drewery wrote:
> On 7/2/2018 6:55 PM, Matt Macy wrote:
>> Author: mmacy
>> Date: Tue Jul 3 01:55:09 2018
>> New Revision: 335879
>> URL: https://svnweb.freebsd.org/changeset/base/335879
>>
>> Log:
>> make critical_{enter, exit} inline
>>
>> Avoid pulling in all of the dependencies by
>> automatically generating a stripped down thread_lite exporting
>> only the fields of interest. The field declarations are type checked
>> against the original and the offsets of the generated result is
>> automatically checked.
>>
>> kib has expressed disagreement and would have preferred to simply
>> use genassym style offsets (which loses type check enforcement).
>> jhb has expressed dislike of it due to header pollution and a
>> duplicate structure. He would have preferred to just have defined
>> thread in _thread.h. Nonetheless, he admits that this is the only
>> viable solution at the moment.
>>
>> The impetus for this came from mjg's D15331:
>> "Inline critical_enter/exit for amd64"
>>
>> Reviewed by: jeff
>> Differential Revision: https://reviews.freebsd.org/D16078
>>
>> Added:
>> head/sys/kern/genoffset.c (contents, props changed)
>> head/sys/kern/genoffset.sh (contents, props changed)
>> head/sys/sys/kpilite.h (contents, props changed)
>> Modified:
>> head/sys/conf/kern.post.mk
>> head/sys/conf/kern.pre.mk
>> head/sys/kern/kern_switch.c
>> head/sys/sys/assym.h
>> head/sys/sys/systm.h
>>
>> Modified: head/sys/conf/kern.post.mk
>> ==
>> --- head/sys/conf/kern.post.mkMon Jul 2 22:59:29 2018
>> (r335878)
>> +++ head/sys/conf/kern.post.mkTue Jul 3 01:55:09 2018
>> (r335879)
>> @@ -185,13 +185,25 @@ hack.pico: Makefile
>> ${CC} ${HACK_EXTRA_FLAGS} -nostdlib hack.c -o hack.pico
>> rm -f hack.c
>>
>> -assym.inc: $S/kern/genassym.sh genassym.o
>> +offset.inc: $S/kern/genoffset.sh genoffset.o
>> + NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genoffset.sh genoffset.o >
>> ${.TARGET}
>> +
>> +genoffset.o: $S/kern/genoffset.c
>> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} $S/kern/genoffset.c
>> +
>> +genoffset_test.c: $S/kern/genoffset.c
>> + cp $S/kern/genoffset.c genoffset_test.c
>> +
>> +genoffset_test.o: genoffset_test.c offset.inc
>> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} -DOFFSET_TEST genoffset_test.c
>> +
>> +assym.inc: $S/kern/genassym.sh genassym.o genoffset_test.o
>> NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genassym.sh genassym.o >
>> ${.TARGET}
>
> What's genoffset_test? Nothing seems to use it.
>
> --
> Regards,
> Bryan Drewery
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r335879 - in head/sys: conf kern sys
On 7/2/2018 6:55 PM, Matt Macy wrote:
> Author: mmacy
> Date: Tue Jul 3 01:55:09 2018
> New Revision: 335879
> URL: https://svnweb.freebsd.org/changeset/base/335879
>
> Log:
> make critical_{enter, exit} inline
>
> Avoid pulling in all of the dependencies by
> automatically generating a stripped down thread_lite exporting
> only the fields of interest. The field declarations are type checked
> against the original and the offsets of the generated result is
> automatically checked.
>
> kib has expressed disagreement and would have preferred to simply
> use genassym style offsets (which loses type check enforcement).
> jhb has expressed dislike of it due to header pollution and a
> duplicate structure. He would have preferred to just have defined
> thread in _thread.h. Nonetheless, he admits that this is the only
> viable solution at the moment.
>
> The impetus for this came from mjg's D15331:
> "Inline critical_enter/exit for amd64"
>
> Reviewed by: jeff
> Differential Revision: https://reviews.freebsd.org/D16078
>
> Added:
> head/sys/kern/genoffset.c (contents, props changed)
> head/sys/kern/genoffset.sh (contents, props changed)
> head/sys/sys/kpilite.h (contents, props changed)
> Modified:
> head/sys/conf/kern.post.mk
> head/sys/conf/kern.pre.mk
> head/sys/kern/kern_switch.c
> head/sys/sys/assym.h
> head/sys/sys/systm.h
>
> Modified: head/sys/conf/kern.post.mk
> ==
> --- head/sys/conf/kern.post.mkMon Jul 2 22:59:29 2018
> (r335878)
> +++ head/sys/conf/kern.post.mkTue Jul 3 01:55:09 2018
> (r335879)
> @@ -185,13 +185,25 @@ hack.pico: Makefile
> ${CC} ${HACK_EXTRA_FLAGS} -nostdlib hack.c -o hack.pico
> rm -f hack.c
>
> -assym.inc: $S/kern/genassym.sh genassym.o
> +offset.inc: $S/kern/genoffset.sh genoffset.o
> + NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genoffset.sh genoffset.o >
> ${.TARGET}
> +
> +genoffset.o: $S/kern/genoffset.c
> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} $S/kern/genoffset.c
> +
> +genoffset_test.c: $S/kern/genoffset.c
> + cp $S/kern/genoffset.c genoffset_test.c
> +
> +genoffset_test.o: genoffset_test.c offset.inc
> + ${CC} -c ${CFLAGS:N-flto:N-fno-common} -DOFFSET_TEST genoffset_test.c
> +
> +assym.inc: $S/kern/genassym.sh genassym.o genoffset_test.o
> NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genassym.sh genassym.o >
> ${.TARGET}
What's genoffset_test? Nothing seems to use it.
--
Regards,
Bryan Drewery
signature.asc
Description: OpenPGP digital signature
Re: svn commit: r335879 - in head/sys: conf kern sys [ broke ci.freebsd.org's FreeBSD-head-amd64-build ]
On 2018-Jul-3, at 10:06 AM, Li-Wen Hsu wrote: > On Tue, Jul 3, 2018 at 5:30 PM Bryan Drewery wrote: >> >> On 7/2/2018 10:46 PM, Mark Millard wrote: >>> -r335879 broke ci.freebsd.org's FreeBSD-head-amd64-build : >>> >>> https://ci.freebsd.org/job/FreeBSD-head-amd64-build/ >>> >>> shows: >>> >>> --- ia32_genassym.o --- >>> In file included from /usr/src/sys/compat/ia32/ia32_genassym.c:6: >>> In file included from /usr/src/sys/sys/systm.h:113: >>> /usr/src/sys/sys/kpilite.h:33:10: fatal error: 'offset.inc' file not found >>> #include "offset.inc" >>> ^~~~ >>> 1 error generated. >>> *** [ia32_genassym.o] Error code 1 >>> >>> Later builds ( -r335880 , -r335881 , -r335882 ) get the same. >>> >>> FreeBSD-head-i386-LINT also fails for such reasons. >>> >> >> r335884 should fix this. > > It seems that amd64 and i386 LINT are still failing. https://ci.freebsd.org/job/FreeBSD-head-amd64-build/ shows that #9303 (-r335884) and later for amd64 are building successfully. But https://ci.freebsd.org/job/FreeBSD-head-i386-LINT/ shows that -r335884 (#6934) and later ( -r335892 so far ) for i386's LINT are still failing with: --- linux_genassym.o --- In file included from /workspace/src/sys/i386/linux/linux_genassym.c:6: In file included from /workspace/src/sys/sys/systm.h:113: /workspace/src/sys/sys/kpilite.h:33:10: fatal error: 'offset.inc' file not found #include "offset.inc" ^~~~ 1 error generated. *** [linux_genassym.o] Error code 1 === Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r335879 - in head/sys: conf kern sys [ broke ci.freebsd.org's FreeBSD-head-amd64-build ]
On Tue, Jul 3, 2018 at 5:30 PM Bryan Drewery wrote: > > On 7/2/2018 10:46 PM, Mark Millard wrote: > > -r335879 broke ci.freebsd.org's FreeBSD-head-amd64-build : > > > > https://ci.freebsd.org/job/FreeBSD-head-amd64-build/ > > > > shows: > > > > --- ia32_genassym.o --- > > In file included from /usr/src/sys/compat/ia32/ia32_genassym.c:6: > > In file included from /usr/src/sys/sys/systm.h:113: > > /usr/src/sys/sys/kpilite.h:33:10: fatal error: 'offset.inc' file not found > > #include "offset.inc" > > ^~~~ > > 1 error generated. > > *** [ia32_genassym.o] Error code 1 > > > > Later builds ( -r335880 , -r335881 , -r335882 ) get the same. > > > > FreeBSD-head-i386-LINT also fails for such reasons. > > > > r335884 should fix this. It seems that amd64 and i386 LINT are still failing. -- Li-Wen Hsu https://lwhsu.org ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r335879 - in head/sys: conf kern sys [ broke ci.freebsd.org's FreeBSD-head-amd64-build ]
On 7/2/2018 10:46 PM, Mark Millard wrote: > -r335879 broke ci.freebsd.org's FreeBSD-head-amd64-build : > > https://ci.freebsd.org/job/FreeBSD-head-amd64-build/ > > shows: > > --- ia32_genassym.o --- > In file included from /usr/src/sys/compat/ia32/ia32_genassym.c:6: > In file included from /usr/src/sys/sys/systm.h:113: > /usr/src/sys/sys/kpilite.h:33:10: fatal error: 'offset.inc' file not found > #include "offset.inc" > ^~~~ > 1 error generated. > *** [ia32_genassym.o] Error code 1 > > Later builds ( -r335880 , -r335881 , -r335882 ) get the same. > > FreeBSD-head-i386-LINT also fails for such reasons. > r335884 should fix this. -- Regards, Bryan Drewery signature.asc Description: OpenPGP digital signature
Re: svn commit: r335879 - in head/sys: conf kern sys [ broke ci.freebsd.org's FreeBSD-head-amd64-build ]
-r335879 broke ci.freebsd.org's FreeBSD-head-amd64-build : https://ci.freebsd.org/job/FreeBSD-head-amd64-build/ shows: --- ia32_genassym.o --- In file included from /usr/src/sys/compat/ia32/ia32_genassym.c:6: In file included from /usr/src/sys/sys/systm.h:113: /usr/src/sys/sys/kpilite.h:33:10: fatal error: 'offset.inc' file not found #include "offset.inc" ^~~~ 1 error generated. *** [ia32_genassym.o] Error code 1 Later builds ( -r335880 , -r335881 , -r335882 ) get the same. FreeBSD-head-i386-LINT also fails for such reasons. === Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r335879 - in head/sys: conf kern sys
Author: mmacy
Date: Tue Jul 3 01:55:09 2018
New Revision: 335879
URL: https://svnweb.freebsd.org/changeset/base/335879
Log:
make critical_{enter, exit} inline
Avoid pulling in all of the dependencies by
automatically generating a stripped down thread_lite exporting
only the fields of interest. The field declarations are type checked
against the original and the offsets of the generated result is
automatically checked.
kib has expressed disagreement and would have preferred to simply
use genassym style offsets (which loses type check enforcement).
jhb has expressed dislike of it due to header pollution and a
duplicate structure. He would have preferred to just have defined
thread in _thread.h. Nonetheless, he admits that this is the only
viable solution at the moment.
The impetus for this came from mjg's D15331:
"Inline critical_enter/exit for amd64"
Reviewed by: jeff
Differential Revision: https://reviews.freebsd.org/D16078
Added:
head/sys/kern/genoffset.c (contents, props changed)
head/sys/kern/genoffset.sh (contents, props changed)
head/sys/sys/kpilite.h (contents, props changed)
Modified:
head/sys/conf/kern.post.mk
head/sys/conf/kern.pre.mk
head/sys/kern/kern_switch.c
head/sys/sys/assym.h
head/sys/sys/systm.h
Modified: head/sys/conf/kern.post.mk
==
--- head/sys/conf/kern.post.mk Mon Jul 2 22:59:29 2018(r335878)
+++ head/sys/conf/kern.post.mk Tue Jul 3 01:55:09 2018(r335879)
@@ -185,13 +185,25 @@ hack.pico: Makefile
${CC} ${HACK_EXTRA_FLAGS} -nostdlib hack.c -o hack.pico
rm -f hack.c
-assym.inc: $S/kern/genassym.sh genassym.o
+offset.inc: $S/kern/genoffset.sh genoffset.o
+ NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genoffset.sh genoffset.o >
${.TARGET}
+
+genoffset.o: $S/kern/genoffset.c
+ ${CC} -c ${CFLAGS:N-flto:N-fno-common} $S/kern/genoffset.c
+
+genoffset_test.c: $S/kern/genoffset.c
+ cp $S/kern/genoffset.c genoffset_test.c
+
+genoffset_test.o: genoffset_test.c offset.inc
+ ${CC} -c ${CFLAGS:N-flto:N-fno-common} -DOFFSET_TEST genoffset_test.c
+
+assym.inc: $S/kern/genassym.sh genassym.o genoffset_test.o
NM='${NM}' NMFLAGS='${NMFLAGS}' sh $S/kern/genassym.sh genassym.o >
${.TARGET}
-genassym.o: $S/$M/$M/genassym.c
+genassym.o: $S/$M/$M/genassym.c offset.inc
${CC} -c ${CFLAGS:N-flto:N-fno-common} $S/$M/$M/genassym.c
-${SYSTEM_OBJS} genassym.o vers.o: opt_global.h
+${SYSTEM_OBJS} genoffset.o genassym.o vers.o: opt_global.h
.if !empty(.MAKE.MODE:Unormal:Mmeta) && empty(.MAKE.MODE:Unormal:Mnofilemon)
_meta_filemon= 1
@@ -213,10 +225,10 @@ _SKIP_DEPEND= 1
.endif
kernel-depend: .depend
-SRCS= assym.inc vnode_if.h ${BEFORE_DEPEND} ${CFILES} \
+SRCS= assym.inc offset.inc vnode_if.h ${BEFORE_DEPEND} ${CFILES} \
${SYSTEM_CFILES} ${GEN_CFILES} ${SFILES} \
${MFILES:T:S/.m$/.h/}
-DEPENDOBJS+= ${SYSTEM_OBJS} genassym.o
+DEPENDOBJS+= ${SYSTEM_OBJS} genassym.o genoffset.o
DEPENDFILES= ${DEPENDOBJS:O:u:C/^/.depend./}
.if ${MAKE_VERSION} < 20160220
DEPEND_MP?=-MP
Modified: head/sys/conf/kern.pre.mk
==
--- head/sys/conf/kern.pre.mk Mon Jul 2 22:59:29 2018(r335878)
+++ head/sys/conf/kern.pre.mk Tue Jul 3 01:55:09 2018(r335879)
@@ -195,7 +195,7 @@ OFEDCFLAGS= ${CFLAGS:N-I*} -DCONFIG_INFINIBAND_USER_ME
OFED_C_NOIMP= ${CC} -c -o ${.TARGET} ${OFEDCFLAGS} ${WERROR} ${PROF}
OFED_C=${OFED_C_NOIMP} ${.IMPSRC}
-GEN_CFILES= $S/$M/$M/genassym.c ${MFILES:T:S/.m$/.c/}
+GEN_CFILES= $S/$M/$M/genassym.c $S/kern/genoffset.c ${MFILES:T:S/.m$/.c/}
SYSTEM_CFILES= config.c env.c hints.c vnode_if.c
SYSTEM_DEP= Makefile ${SYSTEM_OBJS}
SYSTEM_OBJS= locore.o ${MDOBJS} ${OBJS}
Added: head/sys/kern/genoffset.c
==
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ head/sys/kern/genoffset.c Tue Jul 3 01:55:09 2018(r335879)
@@ -0,0 +1,44 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2018, Matthew Macy
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
