| Message ID | 20220209094158.21941-2-sj@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix trivial build errors on -mm tree | expand |
On Wed, 9 Feb 2022, SeongJae Park wrote: > Commit 4b3b8bd6c8287 ("mm/munlock: mlock_page() munlock_page() batch by > pagevec") in -mm tree[1] implements 'mlock_page_drain()' under > CONFIG_MMU only, but the function is used by 'lru_add_drain_cpu()', > which defined outside of CONFIG_MMU. As a result, below build error > occurs. > > /linux/mm/swap.c: In function 'lru_add_drain_cpu': > /linux/mm/swap.c:637:2: error: implicit declaration of function 'mlock_page_drain' [-Werror=implicit-function-declaration] > 637 | mlock_page_drain(cpu); > | ^~~~~~~~~~~~~~~~ > cc1: some warnings being treated as errors > /linux/scripts/Makefile.build:289: recipe for target 'mm/swap.o' failed > > This commit fixes it by implementing no-op 'mlock_page_drain()' for > !CONFIG_MMU case, similar to 'mlock_new_page()'. > > [1] https://www.ozlabs.org/~akpm/mmotm/broken-out/mm-munlock-mlock_page-munlock_page-batch-by-pagevec.patch > > Signed-off-by: SeongJae Park <sj@kernel.org> > --- > mm/internal.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/internal.h b/mm/internal.h > index 0d240e876831..248224369b34 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -508,6 +508,7 @@ static inline void mlock_vma_page(struct page *page, > static inline void munlock_vma_page(struct page *page, > struct vm_area_struct *vma, bool compound) { } > static inline void mlock_new_page(struct page *page) { } > +static inline void mlock_page_drain(int cpu) { } > static inline void vunmap_range_noflush(unsigned long start, unsigned long end) > { > } Thank you, SeongJae, and thank you Geert for reporting. This patch is good as far as it goes, but Andrew, please don't add it right now: I need to think a bit more, and will send another (or Ack SeongJae's) later in the day. The thing is, SeongJae's patch makes me wonder, why did it not need a !CONFIG_MMU definition for need_mlock_page_drain() too? That's because mm/swap.c's call to it is under an #ifdef CONFIG_SMP, and I imagine that CONFIG_MMU=n usually goes along with (but does not necessarily imply?) CONFIG_SMP=n. It'll be safer to add a need_mlock_page_drain() stub too. But more seriously, is the mlock_page_drain() going to be called when it's wanted, when CONFIG_SMP is not set? I had forgotten how mm/swap.c goes in different directions (for some things but not for others) according to CONFIG_SMP. I'll look into it and come back later. Thanks, Hugh
Hi Hugh, On Wed, Feb 9, 2022 at 4:38 PM Hugh Dickins <hughd@google.com> wrote: > The thing is, SeongJae's patch makes me wonder, why did it not need a > !CONFIG_MMU definition for need_mlock_page_drain() too? That's because > mm/swap.c's call to it is under an #ifdef CONFIG_SMP, and I imagine that > CONFIG_MMU=n usually goes along with (but does not necessarily imply?) > CONFIG_SMP=n. It'll be safer to add a need_mlock_page_drain() stub too. RISC-V K210 is SMP without MMU. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, 9 Feb 2022, Geert Uytterhoeven wrote: > On Wed, Feb 9, 2022 at 4:38 PM Hugh Dickins <hughd@google.com> wrote: > > The thing is, SeongJae's patch makes me wonder, why did it not need a > > !CONFIG_MMU definition for need_mlock_page_drain() too? That's because > > mm/swap.c's call to it is under an #ifdef CONFIG_SMP, and I imagine that > > CONFIG_MMU=n usually goes along with (but does not necessarily imply?) > > CONFIG_SMP=n. It'll be safer to add a need_mlock_page_drain() stub too. > > RISC-V K210 is SMP without MMU. Thanks Geert, that makes it very clear that we also want the stub for need_mlock_page_drain(). I'll follow now with update of SeongJae's patch. My fear of wider implications of not having CONFIG_SMP turned out to be a false alarm. The UP lru_add_drain_cpu() calls mlock_page_drain() just like the SMP one does: the difference between them is merely that the UP case doesn't need an efficient way for asking in advance whether drain on another cpu will be needed. Hugh
diff --git a/mm/internal.h b/mm/internal.h index 0d240e876831..248224369b34 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -508,6 +508,7 @@ static inline void mlock_vma_page(struct page *page, static inline void munlock_vma_page(struct page *page, struct vm_area_struct *vma, bool compound) { } static inline void mlock_new_page(struct page *page) { } +static inline void mlock_page_drain(int cpu) { } static inline void vunmap_range_noflush(unsigned long start, unsigned long end) { }
Commit 4b3b8bd6c8287 ("mm/munlock: mlock_page() munlock_page() batch by pagevec") in -mm tree[1] implements 'mlock_page_drain()' under CONFIG_MMU only, but the function is used by 'lru_add_drain_cpu()', which defined outside of CONFIG_MMU. As a result, below build error occurs. /linux/mm/swap.c: In function 'lru_add_drain_cpu': /linux/mm/swap.c:637:2: error: implicit declaration of function 'mlock_page_drain' [-Werror=implicit-function-declaration] 637 | mlock_page_drain(cpu); | ^~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors /linux/scripts/Makefile.build:289: recipe for target 'mm/swap.o' failed This commit fixes it by implementing no-op 'mlock_page_drain()' for !CONFIG_MMU case, similar to 'mlock_new_page()'. [1] https://www.ozlabs.org/~akpm/mmotm/broken-out/mm-munlock-mlock_page-munlock_page-batch-by-pagevec.patch Signed-off-by: SeongJae Park <sj@kernel.org> --- mm/internal.h | 1 + 1 file changed, 1 insertion(+)