Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210430195232.30491-1-michel@lespinasse.org> <20210430224649.GA29203@lespinasse.org>
 <20210503181118.GA21048@lespinasse.org> <20210517175750.GJ4441@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20210517175750.GJ4441@paulmck-ThinkPad-P17-Gen-1>
From:   Suren Baghdasaryan <surenb@google.com>
Date:   Thu, 20 May 2021 15:10:24 -0700
Message-ID: <CAJuCfpHD=GN2UMhbAhpp+UfvF0doBWcZDNx+u4RzcDezUW2+0g@mail.gmail.com>
Subject: Re: [PATCH 00/29] Speculative page faults (anon vmas only)
To:     "Paul E . McKenney" <paulmck@kernel.org>
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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, May 17, 2021 at 10:57 AM Paul E. McKenney <paulmck@kernel.org> wrot=
e:
>
> 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 =E2=80=98__mod_lruvec_page_state=E2=
=80=99:
> ./include/linux/vmstat.h:504:24: error: implicit declaration of function =
=E2=80=98page_pgdat=E2=80=99; did you mean =E2=80=98page_private=E2=80=99? =
[-Werror=3Dimplicit-function-declaration]
>   __mod_node_page_state(page_pgdat(page), idx, val);
>                         ^~~~~~~~~~
>                         page_private
> ./include/linux/vmstat.h:504:24: warning: passing argument 1 of =E2=80=98=
__mod_node_page_state=E2=80=99 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 =E2=80=98struct pglist_da=
ta *=E2=80=99 but argument is of type =E2=80=98int=E2=80=99
>  void __mod_node_page_state(struct pglist_data *, enum node_stat_item ite=
m, long);
>                             ^~~~~~~~~~~~~~~~~~~~
> ./include/linux/vmstat.h: In function =E2=80=98mod_lruvec_page_state=E2=
=80=99:
> ./include/linux/vmstat.h:510:22: warning: passing argument 1 of =E2=80=98=
mod_node_page_state=E2=80=99 makes pointer from integer without a cast [-Wi=
nt-conversion]
>   mod_node_page_state(page_pgdat(page), idx, val);
>                       ^~~~~~~~~~~~~~~~
> ./include/linux/vmstat.h:275:26: note: expected =E2=80=98struct pglist_da=
ta *=E2=80=99 but argument is of type =E2=80=98int=E2=80=99
>  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 =E2=80=98page_=
pgdat=E2=80=99
>  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 =
=E2=80=98page_pgdat=E2=80=99 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:

---
 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.