Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Apr 2009 01:05:20 +0200
From:      Max Laier <max@love2party.net>
To:        freebsd-arch@freebsd.org
Cc:        Robert Watson <rwatson@freebsd.org>
Subject:   Re: Simple #define for cache line size
Message-ID:  <200904140105.20664.max@love2party.net>
In-Reply-To: <alpine.BSF.2.00.0904111700241.19879@fledge.watson.org>
References:  <alpine.BSF.2.00.0904111700241.19879@fledge.watson.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 11 April 2009 18:11:55 Robert Watson wrote:
> Dear all:
>
> We have a number of __aligned() qualifiers scattered around the kernel
> intended to space objects to improve alignment with respect to cache lines.
> This is important for a number of reasons, not least avoiding cache line
> thrashing when using arrays of foo[MAXCPU].  What I'd like to do is provide
> a single compile-time constant, CACHE_LINE_SIZE, defined in
> machine-dependent param.h, to use for spacing such objects, rather than
> hard-coding various values around.  Here are some examples of existing
> spacing attempts:

How much does __aligned(FOO) even achieve?  If we allocate these structs at 
runtime the alignment requirements have to be passed to the allocator as well 
(eg. uma(9)'s align parameter in uma_zcreate).  If my assumption is correct it 
would make sense to have a global symbol that is initialized early in the boot 
process instead of a compile time #define.

In addition it seems that we need to make the define a worst case value to 
assure correctness for static arrays as below.

> i386/include/pcpu.h, line 67
> >   67         char    pc_monitorbuf[128] __aligned(128); /* cache line */ 

like here.  If we define CACHE_LINE to be 64 and happen to run on a CPU that 
has 128 this will no longer work correctly.

> vm/uma_core.c, line 115
>     114 /* The boot-time adjusted value for cache line alignment. */
> >  115 static int uma_align_cache = 64 - 1;

As the comment suggest, the value should be boot time adjusted and used from 
there.

> We could then deploy __aligned(CACHE_LINE_SIZE) in various places, such as
> on the description of per-CPU caches for UMA, to ensure that, for example,
> per-CPU stats for one CPU weren't in the same cache line as stats for other
> CPUs.
>
> NetBSD, FYI, defines CACHE_LINE_SIZE as a global constant in param.h, but
> I'm going with an MD definition as I suspect people will want to do
> different things on different architectures (and there is variation).  I've
> defaulted all architectures to 64 bytes, but I suspect a number would
> prefer to use 32.
>
> Patch below.  There's undoubtably an argument for doing something more
> optimal than what I'm proposing here, but right now I'm just looking for
> something that works, and would be happy to see it replaced with something
> more mature once it's available.
>
> Robert N M Watson
> Computer Laboratory
> University of Cambridge
>
> Index: arm/include/param.h
> ===================================================================
> --- arm/include/param.h	(revision 190941)
> +++ arm/include/param.h	(working copy)
> @@ -81,6 +81,10 @@
>   #define	ALIGNBYTES	_ALIGNBYTES
>   #define	ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define	PAGE_SHIFT	12
>   #define	PAGE_SIZE	(1 << PAGE_SHIFT)	/* Page size */
>   #define	PAGE_MASK	(PAGE_SIZE - 1)
> Index: powerpc/include/param.h
> ===================================================================
> --- powerpc/include/param.h	(revision 190941)
> +++ powerpc/include/param.h	(working copy)
> @@ -79,6 +79,10 @@
>   #define	ALIGNBYTES	_ALIGNBYTES
>   #define	ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define	PAGE_SHIFT	12
>   #define	PAGE_SIZE	(1 << PAGE_SHIFT)	/* Page size */
>   #define	PAGE_MASK	(PAGE_SIZE - 1)
> Index: sparc64/include/param.h
> ===================================================================
> --- sparc64/include/param.h	(revision 190941)
> +++ sparc64/include/param.h	(working copy)
> @@ -71,6 +71,10 @@
>   #define ALIGNBYTES	_ALIGNBYTES
>   #define ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define	PAGE_SHIFT_8K	13
>   #define	PAGE_SIZE_8K	(1L<<PAGE_SHIFT_8K)
>   #define	PAGE_MASK_8K	(PAGE_SIZE_8K-1)
> Index: ia64/include/param.h
> ===================================================================
> --- ia64/include/param.h	(revision 190941)
> +++ ia64/include/param.h	(working copy)
> @@ -99,6 +99,10 @@
>   #define	ALIGN(p)		_ALIGN(p)
>   #define ALIGNED_POINTER(p,t)	_ALIGNED_POINTER(p,t)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #ifndef LOG2_PAGE_SIZE
>   #define	LOG2_PAGE_SIZE		13		/* 8K pages by default. */
>   #endif
> Index: mips/include/param.h
> ===================================================================
> --- mips/include/param.h	(revision 190941)
> +++ mips/include/param.h	(working copy)
> @@ -89,6 +89,10 @@
>   #define	ALIGNBYTES	_ALIGNBYTES
>   #define	ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define	NBPG		4096		/* bytes/page */
>   #define	PGOFSET		(NBPG-1)	/* byte offset into page */
>   #define	PGSHIFT		12		/* LOG2(NBPG) */
> Index: sun4v/include/param.h
> ===================================================================
> --- sun4v/include/param.h	(revision 190941)
> +++ sun4v/include/param.h	(working copy)
> @@ -71,6 +71,10 @@
>   #define ALIGNBYTES	_ALIGNBYTES
>   #define ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define	PAGE_SHIFT_8K	13
>   #define	PAGE_SIZE_8K	(1L<<PAGE_SHIFT_8K)
>   #define	PAGE_MASK_8K	(PAGE_SIZE_8K-1)
> Index: i386/include/param.h
> ===================================================================
> --- i386/include/param.h	(revision 190941)
> +++ i386/include/param.h	(working copy)
> @@ -74,6 +74,10 @@
>   #define ALIGNBYTES	_ALIGNBYTES
>   #define ALIGN(p)	_ALIGN(p)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
> +
>   #define PAGE_SHIFT	12		/* LOG2(PAGE_SIZE) */
>   #define PAGE_SIZE	(1<<PAGE_SHIFT)	/* bytes/page */
>   #define PAGE_MASK	(PAGE_SIZE-1)
> Index: amd64/include/param.h
> ===================================================================
> --- amd64/include/param.h	(revision 190941)
> +++ amd64/include/param.h	(working copy)
> @@ -89,6 +89,9 @@
>   #define	ALIGN(p)		_ALIGN(p)
>   #define	ALIGNED_POINTER(p,t)	_ALIGNED_POINTER(p,t)
>
> +#ifndef CACHE_LINE_SIZE
> +#define	CACHE_LINE_SIZE	64
> +#endif
>
>   /* Size of the level 1 page table units */
>   #define NPTEPG		(PAGE_SIZE/(sizeof (pt_entry_t)))
> _______________________________________________
> freebsd-arch@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
>
>
> !DSPAM:49e0c16232832046891198!

-- 
/"\  Best regards,                      | mlaier@freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier@EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200904140105.20664.max>