There are three users (mmzone.h, memcontrol.h, page_counter.h) using
similar code for forcing cacheline padding between fields of different
structures. Dedup that code.
Signed-off-by: Shakeel Butt <[email protected]>
Suggested-by: Feng Tang <[email protected]>
---
Changes since v1:
- add semicolons in mmzone.h
include/linux/cache.h | 13 +++++++++++++
include/linux/memcontrol.h | 13 ++-----------
include/linux/mmzone.h | 24 +++++-------------------
include/linux/page_counter.h | 13 ++-----------
4 files changed, 22 insertions(+), 41 deletions(-)
diff --git a/include/linux/cache.h b/include/linux/cache.h
index d742c57eaee5..5da1bbd96154 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -85,4 +85,17 @@
#define cache_line_size() L1_CACHE_BYTES
#endif
+/*
+ * Helper to add padding within a struct to ensure data fall into separate
+ * cachelines.
+ */
+#if defined(CONFIG_SMP)
+struct cacheline_padding {
+ char x[0];
+} ____cacheline_internodealigned_in_smp;
+#define CACHELINE_PADDING(name) struct cacheline_padding name
+#else
+#define CACHELINE_PADDING(name)
+#endif
+
#endif /* __LINUX_CACHE_H */
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 344022f102c2..60545e4a1c03 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -185,15 +185,6 @@ struct mem_cgroup_thresholds {
struct mem_cgroup_threshold_ary *spare;
};
-#if defined(CONFIG_SMP)
-struct memcg_padding {
- char x[0];
-} ____cacheline_internodealigned_in_smp;
-#define MEMCG_PADDING(name) struct memcg_padding name
-#else
-#define MEMCG_PADDING(name)
-#endif
-
/*
* Remember four most recent foreign writebacks with dirty pages in this
* cgroup. Inode sharing is expected to be uncommon and, even if we miss
@@ -304,7 +295,7 @@ struct mem_cgroup {
spinlock_t move_lock;
unsigned long move_lock_flags;
- MEMCG_PADDING(_pad1_);
+ CACHELINE_PADDING(_pad1_);
/* memory.stat */
struct memcg_vmstats vmstats;
@@ -326,7 +317,7 @@ struct mem_cgroup {
struct list_head objcg_list;
#endif
- MEMCG_PADDING(_pad2_);
+ CACHELINE_PADDING(_pad2_);
/*
* set > 0 if pages under this cgroup are moving to other cgroup.
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 14919b2fb5f5..b30f5f0dd13f 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -121,20 +121,6 @@ static inline bool free_area_empty(struct free_area *area, int migratetype)
struct pglist_data;
-/*
- * Add a wild amount of padding here to ensure data 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_internodealigned_in_smp;
-#define ZONE_PADDING(name) struct zone_padding name;
-#else
-#define ZONE_PADDING(name)
-#endif
-
#ifdef CONFIG_NUMA
enum numa_stat_item {
NUMA_HIT, /* allocated in intended node */
@@ -837,7 +823,7 @@ struct zone {
int initialized;
/* Write-intensive fields used from the page allocator */
- ZONE_PADDING(_pad1_)
+ CACHELINE_PADDING(_pad1_);
/* free areas of different sizes */
struct free_area free_area[MAX_ORDER];
@@ -849,7 +835,7 @@ struct zone {
spinlock_t lock;
/* Write-intensive fields used by compaction and vmstats. */
- ZONE_PADDING(_pad2_)
+ CACHELINE_PADDING(_pad2_);
/*
* When free pages are below this point, additional steps are taken
@@ -886,7 +872,7 @@ struct zone {
bool contiguous;
- ZONE_PADDING(_pad3_)
+ CACHELINE_PADDING(_pad3_);
/* Zone statistics */
atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
atomic_long_t vm_numa_event[NR_VM_NUMA_EVENT_ITEMS];
@@ -1194,7 +1180,7 @@ typedef struct pglist_data {
#endif /* CONFIG_NUMA */
/* Write-intensive fields used by page reclaim */
- ZONE_PADDING(_pad1_)
+ CACHELINE_PADDING(_pad1_);
#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
/*
@@ -1239,7 +1225,7 @@ typedef struct pglist_data {
struct lru_gen_mm_walk mm_walk;
#endif
- ZONE_PADDING(_pad2_)
+ CACHELINE_PADDING(_pad2_);
/* Per-node vmstats */
struct per_cpu_nodestat __percpu *per_cpu_nodestats;
diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index 78a1c934e416..c141ea9a95ef 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -7,22 +7,13 @@
#include <linux/kernel.h>
#include <asm/page.h>
-#if defined(CONFIG_SMP)
-struct pc_padding {
- char x[0];
-} ____cacheline_internodealigned_in_smp;
-#define PC_PADDING(name) struct pc_padding name
-#else
-#define PC_PADDING(name)
-#endif
-
struct page_counter {
/*
* Make sure 'usage' does not share cacheline with any other field. The
* memcg->memory.usage is a hot member of struct mem_cgroup.
*/
atomic_long_t usage;
- PC_PADDING(_pad1_);
+ CACHELINE_PADDING(_pad1_);
/* effective memory.min and memory.min usage tracking */
unsigned long emin;
@@ -38,7 +29,7 @@ struct page_counter {
unsigned long failcnt;
/* Keep all the read most fields in a separete cacheline. */
- PC_PADDING(_pad2_);
+ CACHELINE_PADDING(_pad2_);
unsigned long min;
unsigned long low;
--
2.37.2.672.g94769d06f0-goog
On Fri, 26 Aug 2022 23:06:42 +0000 Shakeel Butt <[email protected]> wrote:
> There are three users (mmzone.h, memcontrol.h, page_counter.h) using
> similar code for forcing cacheline padding between fields of different
> structures. Dedup that code.
>
> ...
>
> include/linux/cache.h | 13 +++++++++++++
> include/linux/memcontrol.h | 13 ++-----------
> include/linux/mmzone.h | 24 +++++-------------------
> include/linux/page_counter.h | 13 ++-----------
I'm looking for an obvious and reliable way in which those three
headers include cache.h. Seems that mm_types.h's inclusion of cache.h
should be sufficient. Direct inclusion of cache.h would of course be
safest. I guess if it breaks, we'll hear about it ;)
On Fri 26-08-22 23:06:42, Shakeel Butt wrote:
> There are three users (mmzone.h, memcontrol.h, page_counter.h) using
> similar code for forcing cacheline padding between fields of different
> structures. Dedup that code.
>
> Signed-off-by: Shakeel Butt <[email protected]>
> Suggested-by: Feng Tang <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Thanks!
> ---
> Changes since v1:
> - add semicolons in mmzone.h
>
> include/linux/cache.h | 13 +++++++++++++
> include/linux/memcontrol.h | 13 ++-----------
> include/linux/mmzone.h | 24 +++++-------------------
> include/linux/page_counter.h | 13 ++-----------
> 4 files changed, 22 insertions(+), 41 deletions(-)
>
> diff --git a/include/linux/cache.h b/include/linux/cache.h
> index d742c57eaee5..5da1bbd96154 100644
> --- a/include/linux/cache.h
> +++ b/include/linux/cache.h
> @@ -85,4 +85,17 @@
> #define cache_line_size() L1_CACHE_BYTES
> #endif
>
> +/*
> + * Helper to add padding within a struct to ensure data fall into separate
> + * cachelines.
> + */
> +#if defined(CONFIG_SMP)
> +struct cacheline_padding {
> + char x[0];
> +} ____cacheline_internodealigned_in_smp;
> +#define CACHELINE_PADDING(name) struct cacheline_padding name
> +#else
> +#define CACHELINE_PADDING(name)
> +#endif
> +
> #endif /* __LINUX_CACHE_H */
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 344022f102c2..60545e4a1c03 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -185,15 +185,6 @@ struct mem_cgroup_thresholds {
> struct mem_cgroup_threshold_ary *spare;
> };
>
> -#if defined(CONFIG_SMP)
> -struct memcg_padding {
> - char x[0];
> -} ____cacheline_internodealigned_in_smp;
> -#define MEMCG_PADDING(name) struct memcg_padding name
> -#else
> -#define MEMCG_PADDING(name)
> -#endif
> -
> /*
> * Remember four most recent foreign writebacks with dirty pages in this
> * cgroup. Inode sharing is expected to be uncommon and, even if we miss
> @@ -304,7 +295,7 @@ struct mem_cgroup {
> spinlock_t move_lock;
> unsigned long move_lock_flags;
>
> - MEMCG_PADDING(_pad1_);
> + CACHELINE_PADDING(_pad1_);
>
> /* memory.stat */
> struct memcg_vmstats vmstats;
> @@ -326,7 +317,7 @@ struct mem_cgroup {
> struct list_head objcg_list;
> #endif
>
> - MEMCG_PADDING(_pad2_);
> + CACHELINE_PADDING(_pad2_);
>
> /*
> * set > 0 if pages under this cgroup are moving to other cgroup.
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 14919b2fb5f5..b30f5f0dd13f 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -121,20 +121,6 @@ static inline bool free_area_empty(struct free_area *area, int migratetype)
>
> struct pglist_data;
>
> -/*
> - * Add a wild amount of padding here to ensure data 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_internodealigned_in_smp;
> -#define ZONE_PADDING(name) struct zone_padding name;
> -#else
> -#define ZONE_PADDING(name)
> -#endif
> -
> #ifdef CONFIG_NUMA
> enum numa_stat_item {
> NUMA_HIT, /* allocated in intended node */
> @@ -837,7 +823,7 @@ struct zone {
> int initialized;
>
> /* Write-intensive fields used from the page allocator */
> - ZONE_PADDING(_pad1_)
> + CACHELINE_PADDING(_pad1_);
>
> /* free areas of different sizes */
> struct free_area free_area[MAX_ORDER];
> @@ -849,7 +835,7 @@ struct zone {
> spinlock_t lock;
>
> /* Write-intensive fields used by compaction and vmstats. */
> - ZONE_PADDING(_pad2_)
> + CACHELINE_PADDING(_pad2_);
>
> /*
> * When free pages are below this point, additional steps are taken
> @@ -886,7 +872,7 @@ struct zone {
>
> bool contiguous;
>
> - ZONE_PADDING(_pad3_)
> + CACHELINE_PADDING(_pad3_);
> /* Zone statistics */
> atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
> atomic_long_t vm_numa_event[NR_VM_NUMA_EVENT_ITEMS];
> @@ -1194,7 +1180,7 @@ typedef struct pglist_data {
> #endif /* CONFIG_NUMA */
>
> /* Write-intensive fields used by page reclaim */
> - ZONE_PADDING(_pad1_)
> + CACHELINE_PADDING(_pad1_);
>
> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> /*
> @@ -1239,7 +1225,7 @@ typedef struct pglist_data {
> struct lru_gen_mm_walk mm_walk;
> #endif
>
> - ZONE_PADDING(_pad2_)
> + CACHELINE_PADDING(_pad2_);
>
> /* Per-node vmstats */
> struct per_cpu_nodestat __percpu *per_cpu_nodestats;
> diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
> index 78a1c934e416..c141ea9a95ef 100644
> --- a/include/linux/page_counter.h
> +++ b/include/linux/page_counter.h
> @@ -7,22 +7,13 @@
> #include <linux/kernel.h>
> #include <asm/page.h>
>
> -#if defined(CONFIG_SMP)
> -struct pc_padding {
> - char x[0];
> -} ____cacheline_internodealigned_in_smp;
> -#define PC_PADDING(name) struct pc_padding name
> -#else
> -#define PC_PADDING(name)
> -#endif
> -
> struct page_counter {
> /*
> * Make sure 'usage' does not share cacheline with any other field. The
> * memcg->memory.usage is a hot member of struct mem_cgroup.
> */
> atomic_long_t usage;
> - PC_PADDING(_pad1_);
> + CACHELINE_PADDING(_pad1_);
>
> /* effective memory.min and memory.min usage tracking */
> unsigned long emin;
> @@ -38,7 +29,7 @@ struct page_counter {
> unsigned long failcnt;
>
> /* Keep all the read most fields in a separete cacheline. */
> - PC_PADDING(_pad2_);
> + CACHELINE_PADDING(_pad2_);
>
> unsigned long min;
> unsigned long low;
> --
> 2.37.2.672.g94769d06f0-goog
--
Michal Hocko
SUSE Labs
On Sat, Aug 27, 2022 at 07:06:42AM +0800, Shakeel Butt wrote:
> There are three users (mmzone.h, memcontrol.h, page_counter.h) using
> similar code for forcing cacheline padding between fields of different
> structures. Dedup that code.
>
> Signed-off-by: Shakeel Butt <[email protected]>
> Suggested-by: Feng Tang <[email protected]>
Reviewed-by: Feng Tang <[email protected]>
Thanks,
Feng
> ---
> Changes since v1:
> - add semicolons in mmzone.h
>
> include/linux/cache.h | 13 +++++++++++++
> include/linux/memcontrol.h | 13 ++-----------
> include/linux/mmzone.h | 24 +++++-------------------
> include/linux/page_counter.h | 13 ++-----------
> 4 files changed, 22 insertions(+), 41 deletions(-)
[...]