This patch removes the zone padding hack and establishes definitions
in include/linux/cache.h to define the padding within struct zone.
Signed-off-by: Christoph Lameter <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
Index: linux-2.6.11/include/linux/cache.h
===================================================================
--- linux-2.6.11.orig/include/linux/cache.h 2005-03-08 18:40:15.000000000 -0800
+++ linux-2.6.11/include/linux/cache.h 2005-03-14 10:33:45.247701040 -0800
@@ -48,4 +48,12 @@
#endif
#endif
+#ifndef ____cacheline_pad_in_smp
+#if defined(CONFIG_SMP)
+#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
+#else
+#define ____cacheline_pad_in_smp
+#endif
+#endif
+
#endif /* __LINUX_CACHE_H */
Index: linux-2.6.11/include/linux/mmzone.h
===================================================================
--- linux-2.6.11.orig/include/linux/mmzone.h 2005-03-14 10:33:01.037422024 -0800
+++ linux-2.6.11/include/linux/mmzone.h 2005-03-14 10:33:45.248700888 -0800
@@ -28,21 +28,6 @@ struct free_area {
struct pglist_data;
-/*
- * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
- * So add a wild amount of padding here to ensure that they fall into separate
- * cachelines. There are very few zone structures in the machine, so space
- * consumption is not a concern here.
- */
-#if defined(CONFIG_SMP)
-struct zone_padding {
- char x[0];
-} ____cacheline_maxaligned_in_smp;
-#define ZONE_PADDING(name) struct zone_padding name;
-#else
-#define ZONE_PADDING(name)
-#endif
-
struct per_cpu_pages {
int count; /* number of pages in the list */
int low; /* low watermark, refill needed */
@@ -131,7 +116,14 @@ struct zone {
struct free_area free_area[MAX_ORDER];
- ZONE_PADDING(_pad1_)
+ /*
+ * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
+ * So add a wild amount of padding here to ensure that they fall into separate
+ * cachelines. There are very few zone structures in the machine, so space
+ * consumption is not a concern here.
+ */
+
+ ____cacheline_pad_in_smp;
/* Fields commonly accessed by the page reclaim scanner */
spinlock_t lru_lock;
@@ -164,7 +156,7 @@ struct zone {
int prev_priority;
- ZONE_PADDING(_pad2_)
+ ____cacheline_pad_in_smp;
/* Rarely used or read-mostly fields */
/*
Christoph Lameter <[email protected]> wrote:
>
> +#ifndef ____cacheline_pad_in_smp
> +#if defined(CONFIG_SMP)
> +#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
> +#else
> +#define ____cacheline_pad_in_smp
> +#endif
> +#endif
That's going to spit a warning with older gcc's. "warning: unnamed
struct/union that defines no instances".
On Tue, 2005-03-15 at 20:12 -0800, Christoph Lameter wrote:
> This patch removes the zone padding hack and establishes definitions
> in include/linux/cache.h to define the padding within struct zone.
>
> Signed-off-by: Christoph Lameter <[email protected]>
> Signed-off-by: Shai Fultheim <[email protected]>
>
> Index: linux-2.6.11/include/linux/cache.h
> ===================================================================
> --- linux-2.6.11.orig/include/linux/cache.h 2005-03-08 18:40:15.000000000 -0800
> +++ linux-2.6.11/include/linux/cache.h 2005-03-14 10:33:45.247701040 -0800
> @@ -48,4 +48,12 @@
> #endif
> #endif
>
> +#ifndef ____cacheline_pad_in_smp
> +#if defined(CONFIG_SMP)
> +#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
^^^^^^^
Doesn't this add a redundant cacheline if the padding is
previously perfect? Because of the extra byte you're adding?
IIRC, the char x[0]; trick does the job correctly.
On Tue, 15 Mar 2005, Andrew Morton wrote:
> Christoph Lameter <[email protected]> wrote:
> >
> > +#ifndef ____cacheline_pad_in_smp
> > +#if defined(CONFIG_SMP)
> > +#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
> > +#else
> > +#define ____cacheline_pad_in_smp
> > +#endif
> > +#endif
>
> That's going to spit a warning with older gcc's. "warning: unnamed
> struct/union that defines no instances".
>
Is it really that important? If the struct is named then there may be
conflicts if its used repeatedly.
On Wed, 16 Mar 2005, Nick Piggin wrote:
> >
> > +#ifndef ____cacheline_pad_in_smp
> > +#if defined(CONFIG_SMP)
> > +#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
> ^^^^^^^
>
> Doesn't this add a redundant cacheline if the padding is
> previously perfect? Because of the extra byte you're adding?
>
> IIRC, the char x[0]; trick does the job correctly.
Good idea.
This patch removes the zone padding hack and establishes definitions
in include/linux/cache.h to define the padding within struct zone.
Signed-off-by: Christoph Lameter <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>
Index: linux-2.6.11/include/linux/cache.h
===================================================================
--- linux-2.6.11.orig/include/linux/cache.h 2005-03-08 18:40:15.000000000 -0800
+++ linux-2.6.11/include/linux/cache.h 2005-03-14 10:33:45.247701040 -0800
@@ -48,4 +48,12 @@
#endif
#endif
+#ifndef ____cacheline_pad_in_smp
+#if defined(CONFIG_SMP)
+#define ____cacheline_pad_in_smp struct { char x[0]; } ____cacheline_maxaligned_in_smp
+#else
+#define ____cacheline_pad_in_smp
+#endif
+#endif
+
#endif /* __LINUX_CACHE_H */
Index: linux-2.6.11/include/linux/mmzone.h
===================================================================
--- linux-2.6.11.orig/include/linux/mmzone.h 2005-03-14 10:33:01.037422024 -0800
+++ linux-2.6.11/include/linux/mmzone.h 2005-03-14 10:33:45.248700888 -0800
@@ -28,21 +28,6 @@ struct free_area {
struct pglist_data;
-/*
- * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
- * So add a wild amount of padding here to ensure that they fall into separate
- * cachelines. There are very few zone structures in the machine, so space
- * consumption is not a concern here.
- */
-#if defined(CONFIG_SMP)
-struct zone_padding {
- char x[0];
-} ____cacheline_maxaligned_in_smp;
-#define ZONE_PADDING(name) struct zone_padding name;
-#else
-#define ZONE_PADDING(name)
-#endif
-
struct per_cpu_pages {
int count; /* number of pages in the list */
int low; /* low watermark, refill needed */
@@ -131,7 +116,14 @@ struct zone {
struct free_area free_area[MAX_ORDER];
- ZONE_PADDING(_pad1_)
+ /*
+ * zone->lock and zone->lru_lock are two of the hottest locks in the kernel.
+ * So add a wild amount of padding here to ensure that they fall into separate
+ * cachelines. There are very few zone structures in the machine, so space
+ * consumption is not a concern here.
+ */
+
+ ____cacheline_pad_in_smp;
/* Fields commonly accessed by the page reclaim scanner */
spinlock_t lru_lock;
@@ -164,7 +156,7 @@ struct zone {
int prev_priority;
- ZONE_PADDING(_pad2_)
+ ____cacheline_pad_in_smp;
/* Rarely used or read-mostly fields */
/*
Christoph Lameter <[email protected]> wrote:
>
> On Tue, 15 Mar 2005, Andrew Morton wrote:
>
> > Christoph Lameter <[email protected]> wrote:
> > >
> > > +#ifndef ____cacheline_pad_in_smp
> > > +#if defined(CONFIG_SMP)
> > > +#define ____cacheline_pad_in_smp struct { char x; } ____cacheline_maxaligned_in_smp
> > > +#else
> > > +#define ____cacheline_pad_in_smp
> > > +#endif
> > > +#endif
> >
> > That's going to spit a warning with older gcc's. "warning: unnamed
> > struct/union that defines no instances".
> >
> Is it really that important?
Well, it makes gcc-2.95.x unusable, and a number of people like to use it.
It has not proven too burdensome to support. And we know that if it works
on 2.95.x, it will work on 3.1, 3.2, 3.3, etc.
> If the struct is named then there may be
> conflicts if its used repeatedly.
Hence the "hack" which you just deleted ;)
On Tue, 15 Mar 2005, Andrew Morton wrote:
> > If the struct is named then there may be
> > conflicts if its used repeatedly.
>
> Hence the "hack" which you just deleted ;)
Ok, Master, I see the light....