Date: Mon, 16 Jan 2017 13:46:53 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-arch@freebsd.org Subject: Re: __read_only in the kernel Message-ID: <20170116124653.GB30475@dft-labs.eu> In-Reply-To: <20170113130942.E1016@besplex.bde.org> References: <20161127212503.GA23218@dft-labs.eu> <20161130011033.GA20999@dft-labs.eu> <20161201070246.S1023@besplex.bde.org> <20161229114554.GA29676@dft-labs.eu> <20161229153138.GC29676@dft-labs.eu> <20170113130942.E1016@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jan 13, 2017 at 01:10:33PM +1100, Bruce Evans wrote:
> On Thu, 29 Dec 2016, Mateusz Guzik wrote:
> >Scratch the previous patch. I extended it with __exclusive_cache_line
> >(happy with ideas for a better name) - to be used for something which
> >has to be alone in the cacheline, e.g. an atomic counter.
> >
> >diff --git a/sys/compat/linuxkpi/common/include/linux/compiler.h b/sys/compat/linuxkpi/common/include/linux/compiler.h
> >index c780abc..c32f1fa 100644
> >--- a/sys/compat/linuxkpi/common/include/linux/compiler.h
> >+++ b/sys/compat/linuxkpi/common/include/linux/compiler.h
> >@@ -67,7 +67,6 @@
> >#define typeof(x) __typeof(x)
> >
> >#define uninitialized_var(x) x = x
> >-#define __read_mostly __attribute__((__section__(".data.read_mostly")))
> >#define __always_unused __unused
> >#define __must_check __result_use_check
> >
> >diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
> >index 5d86b03..45685a4 100644
> >--- a/sys/conf/ldscript.amd64
> >+++ b/sys/conf/ldscript.amd64
> >@@ -150,6 +150,17 @@ SECTIONS
> > *(.data .data.* .gnu.linkonce.d.*)
> > KEEP (*(.gnu.linkonce.d.*personality*))
> > }
> >+ . = ALIGN(64);
> >+ .data.read_mostly :
> >+ {
> >+ *(.data.read_mostly)
> >+ }
> >+ . = ALIGN(64);
> >+ .data.exclusive_cache_line :
> >+ {
> >+ *(.data.exclusive_cache_line)
> >+ }
> >+ . = ALIGN(64);
> > .data1 : { *(.data1) }
> > _edata = .; PROVIDE (edata = .);
> > __bss_start = .;
>
> I don't really understand this. Is it still needed? I forget if you
> need separate sections to keep things together for more than the
> purpose of aligning them. I think objects aligned to the cache line
> size or larger can live almost anywhere in the address space without
> making much difference to cache effectiveness on most arches including
> amd64.
>
Without the above, while variables are aligned, they can end up with
unaligned ones added just after them in the same cacheline.
> >diff --git a/sys/dev/drm2/drm_os_freebsd.h b/sys/dev/drm2/drm_os_freebsd.h
> >index dc01c6a..11c9feb 100644
> >--- a/sys/dev/drm2/drm_os_freebsd.h
> >+++ b/sys/dev/drm2/drm_os_freebsd.h
> >@@ -80,7 +80,6 @@ typedef void irqreturn_t;
> >
> >#define __init
> >#define __exit
> >-#define __read_mostly
> >
> >#define BUILD_BUG_ON(x) CTASSERT(!(x))
> >#define BUILD_BUG_ON_NOT_POWER_OF_2(x)
> >diff --git a/sys/sys/systm.h b/sys/sys/systm.h
> >index a1ce9b4..719e063 100644
> >--- a/sys/sys/systm.h
> >+++ b/sys/sys/systm.h
> >@@ -445,4 +445,8 @@ extern void (*softdep_ast_cleanup)(void);
> >
> >void counted_warning(unsigned *counter, const char *msg);
> >
> >+#define __read_mostly __section(".data.read_mostly")
> >+#define __exclusive_cache_line __aligned(CACHE_LINE_SIZE) \
> >+ __section(".data.exclusive_cache_line")
> >+
> >#endif /* !_SYS_SYSTM_H_ */
>
> systm.h is better for the kernel, but will we want this for userland?
>
imho userspace may have their own variants which should not interfere with.
> systm.h is randomly ordered, and appending to it with the new definitions
> inversely ordered improves its randomness. Similarly for style. Indentation
> for multi-line #define's is more random now.
>
Where do you propose moving this? I'm completely indifferent with the
location. Fixed the indentation below.
diff --git a/sys/compat/linuxkpi/common/include/linux/compiler.h b/sys/compat/linuxkpi/common/include/linux/compiler.h
index c780abc..c32f1fa 100644
--- a/sys/compat/linuxkpi/common/include/linux/compiler.h
+++ b/sys/compat/linuxkpi/common/include/linux/compiler.h
@@ -67,7 +67,6 @@
#define typeof(x) __typeof(x)
#define uninitialized_var(x) x = x
-#define __read_mostly __attribute__((__section__(".data.read_mostly")))
#define __always_unused __unused
#define __must_check __result_use_check
diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
index 5d86b03..e64a957 100644
--- a/sys/conf/ldscript.amd64
+++ b/sys/conf/ldscript.amd64
@@ -145,6 +145,17 @@ SECTIONS
.got : { *(.got) }
. = DATA_SEGMENT_RELRO_END (24, .);
.got.plt : { *(.got.plt) }
+ . = ALIGN(64);
+ .data.read_mostly :
+ {
+ *(.data.read_mostly)
+ }
+ . = ALIGN(64);
+ .data.exclusive_cache_line :
+ {
+ *(.data.exclusive_cache_line)
+ }
+ . = ALIGN(64);
.data :
{
*(.data .data.* .gnu.linkonce.d.*)
diff --git a/sys/dev/drm2/drm_os_freebsd.h b/sys/dev/drm2/drm_os_freebsd.h
index dc01c6a..11c9feb 100644
--- a/sys/dev/drm2/drm_os_freebsd.h
+++ b/sys/dev/drm2/drm_os_freebsd.h
@@ -80,7 +80,6 @@ typedef void irqreturn_t;
#define __init
#define __exit
-#define __read_mostly
#define BUILD_BUG_ON(x) CTASSERT(!(x))
#define BUILD_BUG_ON_NOT_POWER_OF_2(x)
diff --git a/sys/sys/systm.h b/sys/sys/systm.h
index a1ce9b4..418998e 100644
--- a/sys/sys/systm.h
+++ b/sys/sys/systm.h
@@ -445,4 +445,8 @@ extern void (*softdep_ast_cleanup)(void);
void counted_warning(unsigned *counter, const char *msg);
+#define __read_mostly __section(".data.read_mostly")
+#define __exclusive_cache_line __aligned(CACHE_LINE_SIZE) \
+ __section(".data.exclusive_cache_line")
+
#endif /* !_SYS_SYSTM_H_ */
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170116124653.GB30475>