Re: [PATCH] Replace zone padding with a definition in cache.h
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Replace zone padding with a definition in cache.h
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 ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Replace zone padding with a definition in cache.h
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.0
-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.h2005-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_areafree_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 */
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Replace zone padding with a definition in cache.h
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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Replace zone padding with a definition in cache.h
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.0
> -0800
> +++ linux-2.6.11/include/linux/cache.h2005-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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Replace zone padding with a definition in cache.h
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".
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
