Re: [PATCH 00/29] Speculative page faults (anon vmas only)

From: "Paul E. McKenney" <paulmck@kernel.org>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Michel Lespinasse <michel@lespinasse.org>,
	Linux-MM <linux-mm@kvack.org>,
	Linux-Kernel <linux-kernel@vger.kernel.org>,
	Laurent Dufour <ldufour@linux.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Michal Hocko <mhocko@suse.com>,
	Matthew Wilcox <willy@infradead.org>,
	Rik van Riel <riel@surriel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joel Fernandes <joelaf@google.com>,
	Andy Lutomirski <luto@kernel.org>
Subject: Re: [PATCH 00/29] Speculative page faults (anon vmas only)
Date: Tue, 1 Jun 2021 13:23:34 -0700	[thread overview]
Message-ID: <20210601202334.GS4397@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <CAJuCfpHD=GN2UMhbAhpp+UfvF0doBWcZDNx+u4RzcDezUW2+0g@mail.gmail.com>

On Thu, May 20, 2021 at 03:10:24PM -0700, Suren Baghdasaryan wrote:
> On Mon, May 17, 2021 at 10:57 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, May 03, 2021 at 11:11:18AM -0700, Michel Lespinasse wrote:
> > > On Fri, Apr 30, 2021 at 03:46:49PM -0700, Michel Lespinasse wrote:
> > > > I- Maple tree
> > > >
> > > > I do not think there is any fundamental conflict between the maple
> > > > tree patches currently being considered, and this patchset.
> > > > I actually have a (very lightly tested) tree merging the two together,
> > > > which was a fairly easy merge. For those interested, I made this
> > > > available at my github, as the v5.12-maple-spf branch.
> > >
> > > People were still confused about it, so the instructions to fetch this are:
> > > git fetch https://github.com/lespinasse/linux.git v5.12-maple-spf
> >
> > Finally getting around to actually testing this, apologies for the
> > delay!
> >
> > Just checking to see if I am in the right place.  The warning below is
> > easily fixed, but I figured that I should check.
> >
> >                                                         Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> >   CC      arch/x86/kernel/asm-offsets.s
> > In file included from ./include/linux/mmap_lock.h:10,
> >                  from ./include/linux/mm.h:18,
> >                  from ./include/linux/kallsyms.h:12,
> >                  from ./include/linux/bpf.h:20,
> >                  from ./include/linux/bpf-cgroup.h:5,
> >                  from ./include/linux/cgroup-defs.h:22,
> >                  from ./include/linux/cgroup.h:28,
> >                  from ./include/linux/memcontrol.h:13,
> >                  from ./include/linux/swap.h:9,
> >                  from ./include/linux/suspend.h:5,
> >                  from arch/x86/kernel/asm-offsets.c:13:
> > ./include/linux/vmstat.h: In function ‘__mod_lruvec_page_state’:
> > ./include/linux/vmstat.h:504:24: error: implicit declaration of function ‘page_pgdat’; did you mean ‘page_private’? [-Werror=implicit-function-declaration]
> >   __mod_node_page_state(page_pgdat(page), idx, val);
> >                         ^~~~~~~~~~
> >                         page_private
> > ./include/linux/vmstat.h:504:24: warning: passing argument 1 of ‘__mod_node_page_state’ makes pointer from integer without a cast [-Wint-conversion]
> >   __mod_node_page_state(page_pgdat(page), idx, val);
> >                         ^~~~~~~~~~~~~~~~
> > ./include/linux/vmstat.h:267:28: note: expected ‘struct pglist_data *’ but argument is of type ‘int’
> >  void __mod_node_page_state(struct pglist_data *, enum node_stat_item item, long);
> >                             ^~~~~~~~~~~~~~~~~~~~
> > ./include/linux/vmstat.h: In function ‘mod_lruvec_page_state’:
> > ./include/linux/vmstat.h:510:22: warning: passing argument 1 of ‘mod_node_page_state’ makes pointer from integer without a cast [-Wint-conversion]
> >   mod_node_page_state(page_pgdat(page), idx, val);
> >                       ^~~~~~~~~~~~~~~~
> > ./include/linux/vmstat.h:275:26: note: expected ‘struct pglist_data *’ but argument is of type ‘int’
> >  void mod_node_page_state(struct pglist_data *, enum node_stat_item, long);
> >                           ^~~~~~~~~~~~~~~~~~~~
> > In file included from ./include/linux/kallsyms.h:12,
> >                  from ./include/linux/bpf.h:20,
> >                  from ./include/linux/bpf-cgroup.h:5,
> >                  from ./include/linux/cgroup-defs.h:22,
> >                  from ./include/linux/cgroup.h:28,
> >                  from ./include/linux/memcontrol.h:13,
> >                  from ./include/linux/swap.h:9,
> >                  from ./include/linux/suspend.h:5,
> >                  from arch/x86/kernel/asm-offsets.c:13:
> > ./include/linux/mm.h: At top level:
> > ./include/linux/mm.h:1568:26: error: conflicting types for ‘page_pgdat’
> >  static inline pg_data_t *page_pgdat(const struct page *page)
> >                           ^~~~~~~~~~
> > In file included from ./include/linux/mmap_lock.h:10,
> >                  from ./include/linux/mm.h:18,
> >                  from ./include/linux/kallsyms.h:12,
> >                  from ./include/linux/bpf.h:20,
> >                  from ./include/linux/bpf-cgroup.h:5,
> >                  from ./include/linux/cgroup-defs.h:22,
> >                  from ./include/linux/cgroup.h:28,
> >                  from ./include/linux/memcontrol.h:13,
> >                  from ./include/linux/swap.h:9,
> >                  from ./include/linux/suspend.h:5,
> >                  from arch/x86/kernel/asm-offsets.c:13:
> > ./include/linux/vmstat.h:504:24: note: previous implicit declaration of ‘page_pgdat’ was here
> >   __mod_node_page_state(page_pgdat(page), idx, val);
> >                         ^~~~~~~~~~
> > cc1: some warnings being treated as errors
> 
> Hi Paul,
> I promised you to look into this but somehow forgot to reply, sorry
> about that. The issue is the new "#include <linux/mm_types.h>" in mm.h
> which causes page_pgdat() usage before it is defined:
> 
> mm.h includes mm_types.h
> mm_types.h includes vmstat.h
> vmstat.h uses page_pgdat()
> mm.h defines page_pgdat()
> 
> Not sure if this is the best way to fix it but this worked fine for me:

And it did fix the build error for me as well, thank you!

							Thanx, Paul

> ---
>  include/linux/mmap_lock.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 98f24a9910a9..13d4a706c0eb 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -7,7 +7,7 @@
>  #include <linux/rwsem.h>
>  #include <linux/tracepoint-defs.h>
>  #include <linux/types.h>
> -#include <linux/vmstat.h>
> +#include <linux/vm_event_item.h>
> 
>  #ifdef CONFIG_SPECULATIVE_PAGE_FAULT
>  #define MMAP_LOCK_SEQ_INITIALIZER(name) \
> @@ -113,6 +113,8 @@ static inline bool __mmap_seq_read_check(struct
> mm_struct *mm,
>  }
> 
>  #ifdef CONFIG_SPECULATIVE_PAGE_FAULT_STATS
> +static inline void count_vm_event(enum vm_event_item item);
> +
>  static inline bool mmap_seq_read_check(struct mm_struct *mm, unsigned long seq,
>          enum vm_event_item fail_event)
>  {
> --
> 
> Thanks,
> Suren.