LKML Archive on lore.kernel.org
 help / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Guenter Roeck <private@roeck-us.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: hexagon: build error in -next due to 'mm: memcontrol: per-lruvec stats infrastructure'
Date: Fri, 16 Jun 2017 16:15:23 -0400
Message-ID: <20170616201523.GA11016@cmpxchg.org> (raw)
In-Reply-To: <20170616121453.34d92894ee9da8af895c4208@linux-foundation.org>

On Fri, Jun 16, 2017 at 12:14:53PM -0700, Andrew Morton wrote:
> On Fri, 16 Jun 2017 14:49:51 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:
> 
> > On Wed, Jun 14, 2017 at 12:26:46AM -0700, Guenter Roeck wrote:
> > > Hi,
> > > 
> > > I see the following build error in -next when building hexagon images.
> > > 
> > >   CC      arch/hexagon/kernel/asm-offsets.s
> > > In file included from ./include/linux/memcontrol.h:30:0,
> > >                  from ./include/linux/swap.h:8,
> > >                  from ./arch/hexagon/include/asm/pgtable.h:27,
> > >                  from ./include/linux/mm.h:70,
> > >                  from arch/hexagon/kernel/asm-offsets.c:28:
> > > ./include/linux/vmstat.h: In function '__inc_zone_page_state':
> > > ./include/linux/vmstat.h:294:2: error: implicit declaration of function 'page_zone' [-Werror=implicit-function-declaration]
> > > ./include/linux/vmstat.h:294:2: warning: passing argument 1 of '__inc_zone_state' makes pointer from integer without a cast [enabled by default]
> > > ./include/linux/vmstat.h:267:20: note: expected 'struct zone *' but argument is of type 'int'
> > 
> > vmstat.h depends on definitions in mm.h, but mm.h through the above
> > chain includes vmstat.h first. It worked in my x86 test because x86
> > pgtable.h doesn't include swap.h.
> > 
> > The headers are a bit of a mess. memcontrol.h is supposed to be a
> > lower level header than mm.h and vmstat.h, yet the new accounting
> > functions depend on mm.h definitions.
> > 
> > Let's move the lruvec accounting infra to vmstat.h and shuffle
> > memcontrol.h into the stack under mm.h and vmstat.h.
> > 
> > Does the following fix the hexagon build?
> 
> This breaks x86_64 allnoconfig.
> 
> arch/x86/mm/pat.c:734: error: redefinition of 'arch_io_reserve_memtype_wc'
> ./include/linux/io.h:175: note: previous definition of 'arch_io_reserve_memtype_wc' was here
> arch/x86/mm/pat.c:742: error: redefinition of 'arch_io_free_memtype_wc'
> ./include/linux/io.h:181: note: previous definition of 'arch_io_free_memtype_wc' was here

wat:

/home/hannes/src/linux/linux/arch/x86/mm/pat.c:734:5: error: redefinition of ‘arch_io_reserve_memtype_wc’                                    
 int arch_io_reserve_memtype_wc(resource_size_t start, resource_size_t size)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~                                                                  
In file included from /home/hannes/src/linux/linux/include/linux/irq.h:24:0,                                                                            
                 from /home/hannes/src/linux/linux/arch/x86/include/asm/hardirq.h:5,
                 from /home/hannes/src/linux/linux/include/linux/hardirq.h:8,
                 from /home/hannes/src/linux/linux/include/linux/memcontrol.h:24,
                 from /home/hannes/src/linux/linux/include/linux/vmstat.h:9,
                 from /home/hannes/src/linux/linux/include/linux/mm.h:1032,
                 from /home/hannes/src/linux/linux/include/linux/pfn_t.h:3,
                 from /home/hannes/src/linux/linux/arch/x86/mm/pat.c:15:
/home/hannes/src/linux/linux/include/linux/io.h:175:19: note: previous definition of ‘arch_io_reserve_memtype_wc’ was here
 static inline int arch_io_reserve_memtype_wc(resource_size_t base,
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~

In any case, memcontrol.h doesn't/shouldn't need hardirq.h. When that
include is removed, the below patch compiles on: x86 allno, x86_64
allno, and my regular x86_64 config:

---
Subject: mm-memcontrol-per-lruvec-stats-infrastructure-fix-4

On Wed, Jun 14, 2017 at 12:26:46AM -0700, Guenter Roeck wrote:
> Hi,
>
> I see the following build error in -next when building hexagon images.
>
>   CC      arch/hexagon/kernel/asm-offsets.s
> In file included from ./include/linux/memcontrol.h:30:0,
>                  from ./include/linux/swap.h:8,
>                  from ./arch/hexagon/include/asm/pgtable.h:27,
>                  from ./include/linux/mm.h:70,
>                  from arch/hexagon/kernel/asm-offsets.c:28:
> ./include/linux/vmstat.h: In function '__inc_zone_page_state':
> ./include/linux/vmstat.h:294:2: error: implicit declaration of function 'page_zone' [-Werror=implicit-function-declaration]
> ./include/linux/vmstat.h:294:2: warning: passing argument 1 of '__inc_zone_state' makes pointer from integer without a cast [enabled by default]
> ./include/linux/vmstat.h:267:20: note: expected 'struct zone *' but argument is of type 'int'

vmstat.h depends on definitions in mm.h, but mm.h through the above
chain includes vmstat.h first. It worked in my x86 test because x86
pgtable.h doesn't include swap.h.

The headers are a bit of a mess. memcontrol.h is supposed to be a
lower level header than mm.h and vmstat.h, yet the new accounting
functions depend on mm.h definitions.

Let's move the lruvec accounting infra to vmstat.h and shuffle
memcontrol.h into the stack under mm.h and vmstat.h.

Reported-by: Guenter Roeck <private@roeck-us.net>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index da9360885260..b5a60c0bcb73 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -21,13 +21,10 @@
 #define _LINUX_MEMCONTROL_H
 #include <linux/cgroup.h>
 #include <linux/vm_event_item.h>
-#include <linux/hardirq.h>
 #include <linux/jump_label.h>
 #include <linux/page_counter.h>
 #include <linux/vmpressure.h>
 #include <linux/eventfd.h>
-#include <linux/mm.h>
-#include <linux/vmstat.h>
 #include <linux/writeback.h>
 #include <linux/page-flags.h>
 
@@ -536,78 +533,6 @@ static inline void mod_memcg_page_state(struct page *page,
 		mod_memcg_state(page->mem_cgroup, idx, val);
 }
 
-static inline unsigned long lruvec_page_state(struct lruvec *lruvec,
-					      enum node_stat_item idx)
-{
-	struct mem_cgroup_per_node *pn;
-	long val = 0;
-	int cpu;
-
-	if (mem_cgroup_disabled())
-		return node_page_state(lruvec_pgdat(lruvec), idx);
-
-	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
-	for_each_possible_cpu(cpu)
-		val += per_cpu(pn->lruvec_stat->count[idx], cpu);
-
-	if (val < 0)
-		val = 0;
-
-	return val;
-}
-
-static inline void __mod_lruvec_state(struct lruvec *lruvec,
-				      enum node_stat_item idx, int val)
-{
-	struct mem_cgroup_per_node *pn;
-
-	__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
-	if (mem_cgroup_disabled())
-		return;
-	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
-	__mod_memcg_state(pn->memcg, idx, val);
-	__this_cpu_add(pn->lruvec_stat->count[idx], val);
-}
-
-static inline void mod_lruvec_state(struct lruvec *lruvec,
-				    enum node_stat_item idx, int val)
-{
-	struct mem_cgroup_per_node *pn;
-
-	mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
-	if (mem_cgroup_disabled())
-		return;
-	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
-	mod_memcg_state(pn->memcg, idx, val);
-	this_cpu_add(pn->lruvec_stat->count[idx], val);
-}
-
-static inline void __mod_lruvec_page_state(struct page *page,
-					   enum node_stat_item idx, int val)
-{
-	struct mem_cgroup_per_node *pn;
-
-	__mod_node_page_state(page_pgdat(page), idx, val);
-	if (mem_cgroup_disabled() || !page->mem_cgroup)
-		return;
-	__mod_memcg_state(page->mem_cgroup, idx, val);
-	pn = page->mem_cgroup->nodeinfo[page_to_nid(page)];
-	__this_cpu_add(pn->lruvec_stat->count[idx], val);
-}
-
-static inline void mod_lruvec_page_state(struct page *page,
-					 enum node_stat_item idx, int val)
-{
-	struct mem_cgroup_per_node *pn;
-
-	mod_node_page_state(page_pgdat(page), idx, val);
-	if (mem_cgroup_disabled() || !page->mem_cgroup)
-		return;
-	mod_memcg_state(page->mem_cgroup, idx, val);
-	pn = page->mem_cgroup->nodeinfo[page_to_nid(page)];
-	this_cpu_add(pn->lruvec_stat->count[idx], val);
-}
-
 unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order,
 						gfp_t gfp_mask,
 						unsigned long *total_scanned);
@@ -835,36 +760,6 @@ static inline void mod_memcg_page_state(struct page *page,
 {
 }
 
-static inline unsigned long lruvec_page_state(struct lruvec *lruvec,
-					      enum node_stat_item idx)
-{
-	return node_page_state(lruvec_pgdat(lruvec), idx);
-}
-
-static inline void __mod_lruvec_state(struct lruvec *lruvec,
-				      enum node_stat_item idx, int val)
-{
-	__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
-}
-
-static inline void mod_lruvec_state(struct lruvec *lruvec,
-				    enum node_stat_item idx, int val)
-{
-	mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
-}
-
-static inline void __mod_lruvec_page_state(struct page *page,
-					   enum node_stat_item idx, int val)
-{
-	__mod_node_page_state(page_pgdat(page), idx, val);
-}
-
-static inline void mod_lruvec_page_state(struct page *page,
-					 enum node_stat_item idx, int val)
-{
-	mod_node_page_state(page_pgdat(page), idx, val);
-}
-
 static inline
 unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order,
 					    gfp_t gfp_mask,
@@ -907,30 +802,6 @@ static inline void __dec_memcg_page_state(struct page *page,
 	__mod_memcg_page_state(page, idx, -1);
 }
 
-static inline void __inc_lruvec_state(struct lruvec *lruvec,
-				      enum node_stat_item idx)
-{
-	__mod_lruvec_state(lruvec, idx, 1);
-}
-
-static inline void __dec_lruvec_state(struct lruvec *lruvec,
-				      enum node_stat_item idx)
-{
-	__mod_lruvec_state(lruvec, idx, -1);
-}
-
-static inline void __inc_lruvec_page_state(struct page *page,
-					   enum node_stat_item idx)
-{
-	__mod_lruvec_page_state(page, idx, 1);
-}
-
-static inline void __dec_lruvec_page_state(struct page *page,
-					   enum node_stat_item idx)
-{
-	__mod_lruvec_page_state(page, idx, -1);
-}
-
 static inline void inc_memcg_state(struct mem_cgroup *memcg,
 				   enum memcg_stat_item idx)
 {
@@ -955,30 +826,6 @@ static inline void dec_memcg_page_state(struct page *page,
 	mod_memcg_page_state(page, idx, -1);
 }
 
-static inline void inc_lruvec_state(struct lruvec *lruvec,
-				    enum node_stat_item idx)
-{
-	mod_lruvec_state(lruvec, idx, 1);
-}
-
-static inline void dec_lruvec_state(struct lruvec *lruvec,
-				    enum node_stat_item idx)
-{
-	mod_lruvec_state(lruvec, idx, -1);
-}
-
-static inline void inc_lruvec_page_state(struct page *page,
-					 enum node_stat_item idx)
-{
-	mod_lruvec_page_state(page, idx, 1);
-}
-
-static inline void dec_lruvec_page_state(struct page *page,
-					 enum node_stat_item idx)
-{
-	mod_lruvec_page_state(page, idx, -1);
-}
-
 #ifdef CONFIG_CGROUP_WRITEBACK
 
 struct list_head *mem_cgroup_cgwb_list(struct mem_cgroup *memcg);
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index b3d85f30d424..4c52a9f7b6a4 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -6,6 +6,7 @@
 #include <linux/mmzone.h>
 #include <linux/vm_event_item.h>
 #include <linux/atomic.h>
+#include <linux/memcontrol.h>
 
 extern int sysctl_stat_interval;
 
@@ -350,4 +351,156 @@ static inline void __mod_zone_freepage_state(struct zone *zone, int nr_pages,
 
 extern const char * const vmstat_text[];
 
+#ifdef CONFIG_MEMCG
+static inline unsigned long lruvec_page_state(struct lruvec *lruvec,
+					      enum node_stat_item idx)
+{
+	struct mem_cgroup_per_node *pn;
+	long val = 0;
+	int cpu;
+
+	if (mem_cgroup_disabled())
+		return node_page_state(lruvec_pgdat(lruvec), idx);
+
+	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
+	for_each_possible_cpu(cpu)
+		val += per_cpu(pn->lruvec_stat->count[idx], cpu);
+
+	if (val < 0)
+		val = 0;
+
+	return val;
+}
+
+static inline void __mod_lruvec_state(struct lruvec *lruvec,
+				      enum node_stat_item idx, int val)
+{
+	struct mem_cgroup_per_node *pn;
+
+	__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
+	if (mem_cgroup_disabled())
+		return;
+	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
+	__mod_memcg_state(pn->memcg, idx, val);
+	__this_cpu_add(pn->lruvec_stat->count[idx], val);
+}
+
+static inline void mod_lruvec_state(struct lruvec *lruvec,
+				    enum node_stat_item idx, int val)
+{
+	struct mem_cgroup_per_node *pn;
+
+	mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
+	if (mem_cgroup_disabled())
+		return;
+	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
+	mod_memcg_state(pn->memcg, idx, val);
+	this_cpu_add(pn->lruvec_stat->count[idx], val);
+}
+
+static inline void __mod_lruvec_page_state(struct page *page,
+					   enum node_stat_item idx, int val)
+{
+	struct mem_cgroup_per_node *pn;
+
+	__mod_node_page_state(page_pgdat(page), idx, val);
+	if (mem_cgroup_disabled() || !page->mem_cgroup)
+		return;
+	__mod_memcg_state(page->mem_cgroup, idx, val);
+	pn = page->mem_cgroup->nodeinfo[page_to_nid(page)];
+	__this_cpu_add(pn->lruvec_stat->count[idx], val);
+}
+
+static inline void mod_lruvec_page_state(struct page *page,
+					 enum node_stat_item idx, int val)
+{
+	struct mem_cgroup_per_node *pn;
+
+	mod_node_page_state(page_pgdat(page), idx, val);
+	if (mem_cgroup_disabled() || !page->mem_cgroup)
+		return;
+	mod_memcg_state(page->mem_cgroup, idx, val);
+	pn = page->mem_cgroup->nodeinfo[page_to_nid(page)];
+	this_cpu_add(pn->lruvec_stat->count[idx], val);
+}
+#else /* CONFIG_MEMCG */
+static inline unsigned long lruvec_page_state(struct lruvec *lruvec,
+					      enum node_stat_item idx)
+{
+	return node_page_state(lruvec_pgdat(lruvec), idx);
+}
+
+static inline void __mod_lruvec_state(struct lruvec *lruvec,
+				      enum node_stat_item idx, int val)
+{
+	__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
+}
+
+static inline void mod_lruvec_state(struct lruvec *lruvec,
+				    enum node_stat_item idx, int val)
+{
+	mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
+}
+
+static inline void __mod_lruvec_page_state(struct page *page,
+					   enum node_stat_item idx, int val)
+{
+	__mod_node_page_state(page_pgdat(page), idx, val);
+}
+
+static inline void mod_lruvec_page_state(struct page *page,
+					 enum node_stat_item idx, int val)
+{
+	mod_node_page_state(page_pgdat(page), idx, val);
+}
+#endif /* CONFIG_MEMCG */
+
+static inline void __inc_lruvec_state(struct lruvec *lruvec,
+				      enum node_stat_item idx)
+{
+	__mod_lruvec_state(lruvec, idx, 1);
+}
+
+static inline void __dec_lruvec_state(struct lruvec *lruvec,
+				      enum node_stat_item idx)
+{
+	__mod_lruvec_state(lruvec, idx, -1);
+}
+
+static inline void __inc_lruvec_page_state(struct page *page,
+					   enum node_stat_item idx)
+{
+	__mod_lruvec_page_state(page, idx, 1);
+}
+
+static inline void __dec_lruvec_page_state(struct page *page,
+					   enum node_stat_item idx)
+{
+	__mod_lruvec_page_state(page, idx, -1);
+}
+
+static inline void inc_lruvec_state(struct lruvec *lruvec,
+				    enum node_stat_item idx)
+{
+	mod_lruvec_state(lruvec, idx, 1);
+}
+
+static inline void dec_lruvec_state(struct lruvec *lruvec,
+				    enum node_stat_item idx)
+{
+	mod_lruvec_state(lruvec, idx, -1);
+}
+
+static inline void inc_lruvec_page_state(struct page *page,
+					 enum node_stat_item idx)
+{
+	mod_lruvec_page_state(page, idx, 1);
+}
+
+static inline void dec_lruvec_page_state(struct page *page,
+					 enum node_stat_item idx)
+{
+	mod_lruvec_page_state(page, idx, -1);
+}
+
 #endif /* _LINUX_VMSTAT_H */

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-14  7:26 Guenter Roeck
2017-06-16 18:49 ` Johannes Weiner
2017-06-16 19:14   ` Andrew Morton
2017-06-16 20:15     ` Johannes Weiner [this message]
2017-06-16 20:37       ` Andrew Morton
2017-06-17 18:11       ` mm-memcontrol-per-lruvec-stats-infrastructure-fix-4 kbuild test robot
2017-06-17 18:28       ` mm-memcontrol-per-lruvec-stats-infrastructure-fix-4 kbuild test robot
2017-06-17 15:37 hexagon: build error in -next due to 'mm: memcontrol: per-lruvec stats infrastructure' Guenter Roeck
2017-06-19 15:47 ` Johannes Weiner
2017-06-17 15:40 Guenter Roeck

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170616201523.GA11016@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=private@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox