linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
@ 2020-06-03 23:23 Guenter Roeck
  2020-06-04  8:35 ` Joerg Roedel
  0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2020-06-03 23:23 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Andrew Morton, Andy Lutomirski, Peter Zijlstra, Linus Torvalds,
	linux-kernel

On Mon, Jun 01, 2020 at 09:52:22PM -0700, Joerg Roedel wrote:
> Track at which levels in the page-table entries were modified by
> vmap/vunmap.
> 
> After the page-table has been modified, use that information do decide
> whether the new arch_sync_kernel_mappings() needs to be called.
> 
> [akpm@linux-foundation.org: map_kernel_range_noflush() needs the arch_sync_kernel_mappings() call]
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Acked-by: Andy Lutomirski <luto@kernel.org>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "H . Peter Anvin" <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Link: http://lkml.kernel.org/r/20200515140023.25469-3-joro@8bytes.org
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

This patch causes crashes with all my ppc64 boot tests, in various
locations depending on the platform. Reverting it together with its
companion "mm/ioremap: track which page-table levels were modified"
fixes the problem.

Various logs are at
https://kerneltests.org/builders/qemu-ppc64-master/builds/1442/steps/qemubuildcommand/logs/stdio

Just for completeness, bisect log attached.

Guenter

---
# bad: [f6aee505c71bbb035dde146caf5a6abbf3ccbe47] Merge tag 'x86-timers-2020-06-03' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
# good: [b23c4771ff62de8ca9b5e4a2d64491b2fb6f8f69] Merge tag 'docs-5.8' of git://git.lwn.net/linux
git bisect start 'HEAD' 'b23c4771ff62'
# good: [c41219fda6e04255c44d37fd2c0d898c1c46abf1] Merge tag 'drm-intel-next-fixes-2020-05-20' of git://anongit.freedesktop.org/drm/drm-intel into drm-next
git bisect good c41219fda6e04255c44d37fd2c0d898c1c46abf1
# bad: [1966391fa576e1fb2701be8bcca197d8f72737b7] mm/migrate.c: attach_page_private already does the get_page
git bisect bad 1966391fa576e1fb2701be8bcca197d8f72737b7
# bad: [c5d6c13843880ad0112f0513f3eb041b258be66e] Merge tag 'mmc-v5.8' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc
git bisect bad c5d6c13843880ad0112f0513f3eb041b258be66e
# bad: [94709049fb8442fb2f7b91fbec3c2897a75e18df] Merge branch 'akpm' (patches from Andrew)
git bisect bad 94709049fb8442fb2f7b91fbec3c2897a75e18df
# good: [a29adb6209cead1f6c34a8d72481fb183bfc2d68] mm: rename vmap_page_range to map_kernel_range
git bisect good a29adb6209cead1f6c34a8d72481fb183bfc2d68
# good: [56446efab9ce4961fe0fe6bbc5bc66374b08b9f3] Merge branch 'uaccess.__copy_from_user' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
git bisect good 56446efab9ce4961fe0fe6bbc5bc66374b08b9f3
# good: [17839856fd588f4ab6b789f482ed3ffd7c403e1f] gup: document and work around "COW can break either way" issue
git bisect good 17839856fd588f4ab6b789f482ed3ffd7c403e1f
# good: [ebcdd3074a729f9ba351278e1b42d7ae7fcdf236] arm64: use __vmalloc_node in arch_alloc_vmap_stack
git bisect good ebcdd3074a729f9ba351278e1b42d7ae7fcdf236
# bad: [86cf69f1d893d48fdb0382a940f2523409406423] x86/mm/32: implement arch_sync_kernel_mappings()
git bisect bad 86cf69f1d893d48fdb0382a940f2523409406423
# good: [d8626138009ba58ae2c22356966c2edaa1f1c3b5] mm: add functions to track page directory modifications
git bisect good d8626138009ba58ae2c22356966c2edaa1f1c3b5
# bad: [6c0c7d2b365b21a413f6d75772a8a4a2c7d36916] mm/ioremap: track which page-table levels were modified
git bisect bad 6c0c7d2b365b21a413f6d75772a8a4a2c7d36916
# bad: [2ba3e6947aed9bb9575eb1603c0ac6e39185d32a] mm/vmalloc: track which page-table levels were modified
git bisect bad 2ba3e6947aed9bb9575eb1603c0ac6e39185d32a
# first bad commit: [2ba3e6947aed9bb9575eb1603c0ac6e39185d32a] mm/vmalloc: track which page-table levels were modified

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-03 23:23 [PATCH] mm/vmalloc: track which page-table levels were modified Guenter Roeck
@ 2020-06-04  8:35 ` Joerg Roedel
  2020-06-04 17:16   ` Linus Torvalds
  0 siblings, 1 reply; 14+ messages in thread
From: Joerg Roedel @ 2020-06-04  8:35 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Andrew Morton, Andy Lutomirski, Peter Zijlstra, Linus Torvalds,
	linux-kernel

Hi Guenter,

On Wed, Jun 03, 2020 at 04:23:11PM -0700, Guenter Roeck wrote:
> This patch causes crashes with all my ppc64 boot tests, in various
> locations depending on the platform. Reverting it together with its
> companion "mm/ioremap: track which page-table levels were modified"
> fixes the problem.
> 
> Various logs are at
> https://kerneltests.org/builders/qemu-ppc64-master/builds/1442/steps/qemubuildcommand/logs/stdio

I posted the fix for this already:

	https://lore.kernel.org/lkml/20200604074446.23944-1-joro@8bytes.org/

Regards,

	Joerg


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-04  8:35 ` Joerg Roedel
@ 2020-06-04 17:16   ` Linus Torvalds
  2020-06-04 21:06     ` Andrew Morton
  2020-06-05  8:12     ` Joerg Roedel
  0 siblings, 2 replies; 14+ messages in thread
From: Linus Torvalds @ 2020-06-04 17:16 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Guenter Roeck, Andrew Morton, Andy Lutomirski, Peter Zijlstra,
	Linux Kernel Mailing List

On Thu, Jun 4, 2020 at 1:35 AM Joerg Roedel <jroedel@suse.de> wrote:
>
> I posted the fix for this already:
>
>         https://lore.kernel.org/lkml/20200604074446.23944-1-joro@8bytes.org/

Ugh.

I was going to apply this directly, but as I looked at the patch I
just found it fairly illegible.

Is there some reason why the 5level-fixup.h versions use that
very-hard-to-follow macro, rather than the inline functions that the
main mm.h file uses?

I'm _assuming_ it's because it gets included in some place where not
everything is defined yet, so making it a macro means that it works
(later on) when everything has come together..

But the solution to that would seem to make all the p.._alloc_track()
macros just be in a different header file, and make them be all
together. We already have that

   #if !__ARCH_HAS_5LEVEL_HACK

in linux/mm.h, so it's not like we really have isolated that issue
into just 5level-fixup.h anyway, and creating a new
<linux/pagetable-alloc.h> header that has all the variations in one
place, and that is only included by the two (!) users of these things
would seem to be a good idea regardless.

Because <linux/mm.h> is included by pretty much everything. Why do we
have those alloc_track functions defined in such a common header when
they are _so_ special?

Please? I'd obviously like this to be fixed on ppc asap, but I'd also
like the fix to improve on the current somewhat confusing situation..

For extra point, the p??_alloc_track() functions could even be
generated from a macro pattern, because the pattern is pretty much set
in stone.

I think the only thing that really differs is the types and the
PGTBL_xyz_MODIFIED mask, and which entry is tested for "none" (which
is also the only thing that makes the 5level fixup case different -
no?

                       Linus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-04 17:16   ` Linus Torvalds
@ 2020-06-04 21:06     ` Andrew Morton
  2020-06-04 21:12       ` Linus Torvalds
  2020-06-05  8:12     ` Joerg Roedel
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2020-06-04 21:06 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Joerg Roedel, Guenter Roeck, Andy Lutomirski, Peter Zijlstra,
	Linux Kernel Mailing List

On Thu, 4 Jun 2020 10:16:07 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Jun 4, 2020 at 1:35 AM Joerg Roedel <jroedel@suse.de> wrote:
> >
> > I posted the fix for this already:
> >
> >         https://lore.kernel.org/lkml/20200604074446.23944-1-joro@8bytes.org/
> 
> Ugh.
> 
> I was going to apply this directly, but as I looked at the patch I
> just found it fairly illegible.
> 
> Is there some reason why the 5level-fixup.h versions use that
> very-hard-to-follow macro, rather than the inline functions that the
> main mm.h file uses?
> 
> I'm _assuming_ it's because it gets included in some place where not
> everything is defined yet, so making it a macro means that it works
> (later on) when everything has come together..
> 
> But the solution to that would seem to make all the p.._alloc_track()
> macros just be in a different header file, and make them be all
> together. We already have that
> 
>    #if !__ARCH_HAS_5LEVEL_HACK
> 
> in linux/mm.h, so it's not like we really have isolated that issue
> into just 5level-fixup.h anyway, and creating a new
> <linux/pagetable-alloc.h> header that has all the variations in one
> place, and that is only included by the two (!) users of these things
> would seem to be a good idea regardless.
> 
> Because <linux/mm.h> is included by pretty much everything. Why do we
> have those alloc_track functions defined in such a common header when
> they are _so_ special?
> 
> Please? I'd obviously like this to be fixed on ppc asap, but I'd also
> like the fix to improve on the current somewhat confusing situation..
> 
> For extra point, the p??_alloc_track() functions could even be
> generated from a macro pattern, because the pattern is pretty much set
> in stone.
> 
> I think the only thing that really differs is the types and the
> PGTBL_xyz_MODIFIED mask, and which entry is tested for "none" (which
> is also the only thing that makes the 5level fixup case different -
> no?

As discussed over in
https://lore.kernel.org/linux-mm/20200604164814.GA7600@kernel.org/,
Mike's "mm: remove __ARCH_HAS_5LEVEL_HACK" patchset
(http://lkml.kernel.org/r/20200414153455.21744-1-rppt@kernel.org) is
expected to fix this.  5level-fixup.h gets removed.

I hope to have that patchset sent over later today.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-04 21:06     ` Andrew Morton
@ 2020-06-04 21:12       ` Linus Torvalds
  2020-06-05  8:16         ` Joerg Roedel
  0 siblings, 1 reply; 14+ messages in thread
From: Linus Torvalds @ 2020-06-04 21:12 UTC (permalink / raw)
  To: Andrew Morton, Mike Rapoport
  Cc: Joerg Roedel, Guenter Roeck, Andy Lutomirski, Peter Zijlstra,
	Linux Kernel Mailing List

On Thu, Jun 4, 2020 at 2:06 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> As discussed over in
> https://lore.kernel.org/linux-mm/20200604164814.GA7600@kernel.org/,
> Mike's "mm: remove __ARCH_HAS_5LEVEL_HACK" patchset
> (http://lkml.kernel.org/r/20200414153455.21744-1-rppt@kernel.org) is
> expected to fix this.  5level-fixup.h gets removed.

Ok, even better.

That said, the commentary about "why is p.._alloc_track() in such a
core header file, when it's only used by two special cases" is
probably still true regardless of the 5-level fixup header.. I assume
Mike didn't do those kinds of changes?

Yeah, I'm probably flailing at windmills, but I do dislike how we
often end up just growing the very core headers that get included by
everybody without ever trying to fix that uncontrolled growth..

               Linus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-04 17:16   ` Linus Torvalds
  2020-06-04 21:06     ` Andrew Morton
@ 2020-06-05  8:12     ` Joerg Roedel
  1 sibling, 0 replies; 14+ messages in thread
From: Joerg Roedel @ 2020-06-05  8:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Guenter Roeck, Andrew Morton, Andy Lutomirski, Peter Zijlstra,
	Linux Kernel Mailing List

On Thu, Jun 04, 2020 at 10:16:07AM -0700, Linus Torvalds wrote:
> On Thu, Jun 4, 2020 at 1:35 AM Joerg Roedel <jroedel@suse.de> wrote:
> >
> > I posted the fix for this already:
> >
> >         https://lore.kernel.org/lkml/20200604074446.23944-1-joro@8bytes.org/
> 
> Ugh.
> 
> I was going to apply this directly, but as I looked at the patch I
> just found it fairly illegible.
> 
> Is there some reason why the 5level-fixup.h versions use that
> very-hard-to-follow macro, rather than the inline functions that the
> main mm.h file uses?
> 
> I'm _assuming_ it's because it gets included in some place where not
> everything is defined yet, so making it a macro means that it works
> (later on) when everything has come together..

Exactly, I had all of the p?d_alloc_track() functions as inlines first,
but that broke compilation on some obscure architectures, so I followed
the rule to make them macros when the p?d_alloc() was also a macro and
defined them close together.


	Joerg

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-04 21:12       ` Linus Torvalds
@ 2020-06-05  8:16         ` Joerg Roedel
  2020-06-05 10:00           ` Mike Rapoport
  0 siblings, 1 reply; 14+ messages in thread
From: Joerg Roedel @ 2020-06-05  8:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, Mike Rapoport, Guenter Roeck, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

On Thu, Jun 04, 2020 at 02:12:14PM -0700, Linus Torvalds wrote:
> That said, the commentary about "why is p.._alloc_track() in such a
> core header file, when it's only used by two special cases" is
> probably still true regardless of the 5-level fixup header.. I assume
> Mike didn't do those kinds of changes?

I'll try to move them to a separate header next week. The compile
testing I set up for this patch-set will certainly be helpful for that
:)


	Joerg


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-05  8:16         ` Joerg Roedel
@ 2020-06-05 10:00           ` Mike Rapoport
  2020-06-09 12:10             ` Joerg Roedel
  0 siblings, 1 reply; 14+ messages in thread
From: Mike Rapoport @ 2020-06-05 10:00 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Linus Torvalds, Andrew Morton, Guenter Roeck, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

On Fri, Jun 05, 2020 at 10:16:44AM +0200, Joerg Roedel wrote:
> On Thu, Jun 04, 2020 at 02:12:14PM -0700, Linus Torvalds wrote:
> > That said, the commentary about "why is p.._alloc_track() in such a
> > core header file, when it's only used by two special cases" is
> > probably still true regardless of the 5-level fixup header.. I assume
> > Mike didn't do those kinds of changes?
> 
> I'll try to move them to a separate header next week. The compile
> testing I set up for this patch-set will certainly be helpful for that
> :)

We already have include/asm-generic/pgalloc.h, so maybe something like
that patch below would fork. This is not even compile tested.

diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
index 73f7421413cb..7aa9c98aff47 100644
--- a/include/asm-generic/pgalloc.h
+++ b/include/asm-generic/pgalloc.h
@@ -102,6 +102,66 @@ static inline void pte_free(struct mm_struct *mm, struct page *pte_page)
 	__free_page(pte_page);
 }
 
+static inline p4d_t *p4d_alloc(struct mm_struct *mm, pgd_t *pgd,
+		unsigned long address)
+{
+	return (unlikely(pgd_none(*pgd)) && __p4d_alloc(mm, pgd, address)) ?
+		NULL : p4d_offset(pgd, address);
+}
+
+static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
+		unsigned long address)
+{
+	return (unlikely(p4d_none(*p4d)) && __pud_alloc(mm, p4d, address)) ?
+		NULL : pud_offset(p4d, address);
+}
+
+static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
+				     unsigned long address,
+				     pgtbl_mod_mask *mod_mask)
+
+{
+	if (unlikely(pgd_none(*pgd))) {
+		if (__p4d_alloc(mm, pgd, address))
+			return NULL;
+		*mod_mask |= PGTBL_PGD_MODIFIED;
+	}
+
+	return p4d_offset(pgd, address);
+}
+
+static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
+				     unsigned long address,
+				     pgtbl_mod_mask *mod_mask)
+{
+	if (unlikely(p4d_none(*p4d))) {
+		if (__pud_alloc(mm, p4d, address))
+			return NULL;
+		*mod_mask |= PGTBL_P4D_MODIFIED;
+	}
+
+	return pud_offset(p4d, address);
+}
+
+static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
+{
+	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
+		NULL: pmd_offset(pud, address);
+}
+
+static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud,
+				     unsigned long address,
+				     pgtbl_mod_mask *mod_mask)
+{
+	if (unlikely(pud_none(*pud))) {
+		if (__pmd_alloc(mm, pud, address))
+			return NULL;
+		*mod_mask |= PGTBL_PUD_MODIFIED;
+	}
+
+	return pmd_offset(pud, address);
+}
+
 #endif /* CONFIG_MMU */
 
 #endif /* __ASM_GENERIC_PGALLOC_H */
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 3f1649a8cf55..36ac65f4744f 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -14,7 +14,6 @@
 #include <linux/mmu_notifier.h>
 #include <linux/swap.h>
 #include <linux/hugetlb_inline.h>
-#include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
 #include <asm/cacheflush.h>
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 86adc71a972f..29a3d7c492f0 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2068,69 +2068,6 @@ static inline void mm_dec_nr_ptes(struct mm_struct *mm) {}
 int __pte_alloc(struct mm_struct *mm, pmd_t *pmd);
 int __pte_alloc_kernel(pmd_t *pmd);
 
-#if defined(CONFIG_MMU)
-
-static inline p4d_t *p4d_alloc(struct mm_struct *mm, pgd_t *pgd,
-		unsigned long address)
-{
-	return (unlikely(pgd_none(*pgd)) && __p4d_alloc(mm, pgd, address)) ?
-		NULL : p4d_offset(pgd, address);
-}
-
-static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
-		unsigned long address)
-{
-	return (unlikely(p4d_none(*p4d)) && __pud_alloc(mm, p4d, address)) ?
-		NULL : pud_offset(p4d, address);
-}
-
-static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
-				     unsigned long address,
-				     pgtbl_mod_mask *mod_mask)
-
-{
-	if (unlikely(pgd_none(*pgd))) {
-		if (__p4d_alloc(mm, pgd, address))
-			return NULL;
-		*mod_mask |= PGTBL_PGD_MODIFIED;
-	}
-
-	return p4d_offset(pgd, address);
-}
-
-static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
-				     unsigned long address,
-				     pgtbl_mod_mask *mod_mask)
-{
-	if (unlikely(p4d_none(*p4d))) {
-		if (__pud_alloc(mm, p4d, address))
-			return NULL;
-		*mod_mask |= PGTBL_P4D_MODIFIED;
-	}
-
-	return pud_offset(p4d, address);
-}
-
-static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
-{
-	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
-		NULL: pmd_offset(pud, address);
-}
-
-static inline pmd_t *pmd_alloc_track(struct mm_struct *mm, pud_t *pud,
-				     unsigned long address,
-				     pgtbl_mod_mask *mod_mask)
-{
-	if (unlikely(pud_none(*pud))) {
-		if (__pmd_alloc(mm, pud, address))
-			return NULL;
-		*mod_mask |= PGTBL_PUD_MODIFIED;
-	}
-
-	return pmd_offset(pud, address);
-}
-#endif /* CONFIG_MMU */
-
 #if USE_SPLIT_PTE_PTLOCKS
 #if ALLOC_SPLIT_PTLOCKS
 void __init ptlock_cache_init(void);
diff --git a/lib/ioremap.c b/lib/ioremap.c
index ad485f08173b..357e5909a364 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -13,6 +13,7 @@
 #include <linux/export.h>
 #include <asm/cacheflush.h>
 #include <asm/pgtable.h>
+#include <asm/pgalloc.h>
 
 #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
 static int __read_mostly ioremap_p4d_capable;
diff --git a/mm/mremap.c b/mm/mremap.c
index 7d69e3fe51aa..4a6c4db8747b 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -27,6 +27,7 @@
 
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
+#include <asm/pgalloc.h>
 
 #include "internal.h"
 
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 3091c2ca60df..84a6d08416a2 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -39,6 +39,7 @@
 #include <linux/uaccess.h>
 #include <asm/tlbflush.h>
 #include <asm/shmparam.h>
+#include <asm/pgalloc.h>
 
 #include "internal.h"
 

> 
> 	Joerg
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-05 10:00           ` Mike Rapoport
@ 2020-06-09 12:10             ` Joerg Roedel
  2020-06-09 14:15               ` Guenter Roeck
  2020-06-09 15:24               ` Mike Rapoport
  0 siblings, 2 replies; 14+ messages in thread
From: Joerg Roedel @ 2020-06-09 12:10 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Linus Torvalds, Andrew Morton, Guenter Roeck, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

Hi Mike,

On Fri, Jun 05, 2020 at 01:00:59PM +0300, Mike Rapoport wrote:
> We already have include/asm-generic/pgalloc.h, so maybe something like
> that patch below would fork. This is not even compile tested.
> 
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h

I experimented a bit with your diff, but it turned out that moving the
page-table allocation functions/macros to asm-generic/pgalloc.h does not
work on all architectures.

The reason is that some archs don't use that header at all (e.g. ARC)
and have their own version of the functions defined there. That could
all be made working, but I decided to no open this can of worms for now.

So I sent out a patch which moves the p?d_alloc_track() functions/macros
to a separate header and include it only in mm/vmalloc.c and
lib/ioremap.c. That compiles on all architectures where current Linus'
master also builds (it does not for Alpha, CSky, Mips and Mips64), and
as usual Hexagon and Unicore32 are not tested because I have no working
compiler for those.

Regards,

	Joerg

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-09 12:10             ` Joerg Roedel
@ 2020-06-09 14:15               ` Guenter Roeck
  2020-06-09 16:23                 ` Joerg Roedel
  2020-06-10  7:59                 ` Mike Rapoport
  2020-06-09 15:24               ` Mike Rapoport
  1 sibling, 2 replies; 14+ messages in thread
From: Guenter Roeck @ 2020-06-09 14:15 UTC (permalink / raw)
  To: Joerg Roedel, Mike Rapoport
  Cc: Linus Torvalds, Andrew Morton, Andy Lutomirski, Peter Zijlstra,
	Linux Kernel Mailing List

On 6/9/20 5:10 AM, Joerg Roedel wrote:
> Hi Mike,
> 
> On Fri, Jun 05, 2020 at 01:00:59PM +0300, Mike Rapoport wrote:
>> We already have include/asm-generic/pgalloc.h, so maybe something like
>> that patch below would fork. This is not even compile tested.
>>
>> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> 
> I experimented a bit with your diff, but it turned out that moving the
> page-table allocation functions/macros to asm-generic/pgalloc.h does not
> work on all architectures.
> 
> The reason is that some archs don't use that header at all (e.g. ARC)
> and have their own version of the functions defined there. That could
> all be made working, but I decided to no open this can of worms for now.
> 
> So I sent out a patch which moves the p?d_alloc_track() functions/macros
> to a separate header and include it only in mm/vmalloc.c and
> lib/ioremap.c. That compiles on all architectures where current Linus'
> master also builds (it does not for Alpha, CSky, Mips and Mips64), and
> as usual Hexagon and Unicore32 are not tested because I have no working
> compiler for those.
> 

To build csky images, you have to disable CONFIG_FRAME_POINTER or use a
non-upstream compiler. To build any images reliably, you have to disable
CONFIG_GCC_PLUGIN_RANDSTRUCT or use a version of gcc old enough to not
support it (gcc 6.x is fine). For mips, you have to specify ARCH and
CROSS_COMPILE as environment variables.

alpha is a lost case. The offending commit is 0f1c9688a19 ("tty/sysrq:
alpha: export and use __sysrq_get_key_op()"); it looks like that wasn't
build tested. It can not be reverted easily because of subsequent changes.

I have stopped building unicore32 images since v4.19 since there is
no available compiler that is still supported by the kernel. I am
surprised that support for it has not been removed from the kernel.

I am told that hexagon only supports llvm/clang, and the only version
of gcc that supports it is v4.6.1 (from Sourcery CodeBench Lite
2012.03-66). The minimum version of gcc will be raised to v4.8;
that has already happened in linux-next and will presumably be applied
to mainline shortly. With that, I'll stop build testing hexagon images
as well until I find the time to build a llvm/clang hexagon toolchain.

So you are in good company, and thanks for testing everything else.

Guenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-09 12:10             ` Joerg Roedel
  2020-06-09 14:15               ` Guenter Roeck
@ 2020-06-09 15:24               ` Mike Rapoport
  1 sibling, 0 replies; 14+ messages in thread
From: Mike Rapoport @ 2020-06-09 15:24 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Linus Torvalds, Andrew Morton, Guenter Roeck, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

Hi Joerg,

On Tue, Jun 09, 2020 at 02:10:56PM +0200, Joerg Roedel wrote:
> Hi Mike,
> 
> On Fri, Jun 05, 2020 at 01:00:59PM +0300, Mike Rapoport wrote:
> > We already have include/asm-generic/pgalloc.h, so maybe something like
> > that patch below would fork. This is not even compile tested.
> > 
> > diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> 
> I experimented a bit with your diff, but it turned out that moving the
> page-table allocation functions/macros to asm-generic/pgalloc.h does not
> work on all architectures.
> 
> The reason is that some archs don't use that header at all (e.g. ARC)
> and have their own version of the functions defined there. That could
> all be made working, but I decided to no open this can of worms for now.
> 
> So I sent out a patch which moves the p?d_alloc_track() functions/macros
> to a separate header and include it only in mm/vmalloc.c and
> lib/ioremap.c. 

I'm planning to open this can of worms eventually, but having
p?d_alloc_track() in a separate header wouldn't be the biggest of my
problems :)

> That compiles on all architectures where current Linus'
> master also builds (it does not for Alpha, CSky, Mips and Mips64), and
> as usual Hexagon and Unicore32 are not tested because I have no working
> compiler for those.
> 
> Regards,
> 
> 	Joerg

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-09 14:15               ` Guenter Roeck
@ 2020-06-09 16:23                 ` Joerg Roedel
  2020-06-09 17:07                   ` Guenter Roeck
  2020-06-10  7:59                 ` Mike Rapoport
  1 sibling, 1 reply; 14+ messages in thread
From: Joerg Roedel @ 2020-06-09 16:23 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mike Rapoport, Linus Torvalds, Andrew Morton, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

Hi Guenter,

On Tue, Jun 09, 2020 at 07:15:42AM -0700, Guenter Roeck wrote:
> To build csky images, you have to disable CONFIG_FRAME_POINTER or use a
> non-upstream compiler. To build any images reliably, you have to disable
> CONFIG_GCC_PLUGIN_RANDSTRUCT or use a version of gcc old enough to not
> support it (gcc 6.x is fine). For mips, you have to specify ARCH and
> CROSS_COMPILE as environment variables.

My test setup builds defconfigs for all architectures using the gcc-9.3
based cross-compilers from kernel.org (except the ones I have no
compiler for). That used to work for CSky and MIPS(64) when working on
my vmalloc changes.

On MIPS the build failure looks like some Makefile breakage, but I
didn't dive deeper into that.

For CSky the compiler complains about not supporting '-mbacktrace'.

> alpha is a lost case. The offending commit is 0f1c9688a19 ("tty/sysrq:
> alpha: export and use __sysrq_get_key_op()"); it looks like that wasn't
> build tested. It can not be reverted easily because of subsequent changes.

The below diff fixed the alpha build for me, but I am sure another fix
is already queued somewhere.

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index 6fa802c495b4..8f4ad63a3a9a 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -431,8 +431,13 @@ register_cpus(void)
 arch_initcall(register_cpus);
 
 #ifdef CONFIG_MAGIC_SYSRQ
+static void sysrq_reboot_handler(int unused)
+{
+	machine_halt();
+}
+
 static const struct sysrq_key_op srm_sysrq_reboot_op = {
-	.handler	= machine_halt,
+	.handler	= sysrq_reboot_handler,
 	.help_msg       = "reboot(b)",
 	.action_msg     = "Resetting",
 	.enable_mask    = SYSRQ_ENABLE_BOOT,


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-09 16:23                 ` Joerg Roedel
@ 2020-06-09 17:07                   ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2020-06-09 17:07 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Mike Rapoport, Linus Torvalds, Andrew Morton, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List

On 6/9/20 9:23 AM, Joerg Roedel wrote:
> Hi Guenter,
> 
> On Tue, Jun 09, 2020 at 07:15:42AM -0700, Guenter Roeck wrote:
>> To build csky images, you have to disable CONFIG_FRAME_POINTER or use a
>> non-upstream compiler. To build any images reliably, you have to disable
>> CONFIG_GCC_PLUGIN_RANDSTRUCT or use a version of gcc old enough to not
>> support it (gcc 6.x is fine). For mips, you have to specify ARCH and
>> CROSS_COMPILE as environment variables.
> 
> My test setup builds defconfigs for all architectures using the gcc-9.3
> based cross-compilers from kernel.org (except the ones I have no
> compiler for). That used to work for CSky and MIPS(64) when working on
> my vmalloc changes.
> 
> On MIPS the build failure looks like some Makefile breakage, but I
> didn't dive deeper into that.
> 

Yes, that was discussed elsewhere. A fix has been submitted and should
hopefully find its way into the kernel sometime soon.

> For CSky the compiler complains about not supporting '-mbacktrace'.
> 

Yes, that is why you have to disable CONFIG_FRAME_POINTER.
'-mbacktrace' is not supported in upstream versions of gcc for csky;
it is only supported in the out-of-tree csky toolchain.

>> alpha is a lost case. The offending commit is 0f1c9688a19 ("tty/sysrq:
>> alpha: export and use __sysrq_get_key_op()"); it looks like that wasn't
>> build tested. It can not be reverted easily because of subsequent changes.
> 
> The below diff fixed the alpha build for me, but I am sure another fix
> is already queued somewhere.
> 

Let's hope it is.

Thanks,
Guenter

> diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
> index 6fa802c495b4..8f4ad63a3a9a 100644
> --- a/arch/alpha/kernel/setup.c
> +++ b/arch/alpha/kernel/setup.c
> @@ -431,8 +431,13 @@ register_cpus(void)
>  arch_initcall(register_cpus);
>  
>  #ifdef CONFIG_MAGIC_SYSRQ
> +static void sysrq_reboot_handler(int unused)
> +{
> +	machine_halt();
> +}
> +
>  static const struct sysrq_key_op srm_sysrq_reboot_op = {
> -	.handler	= machine_halt,
> +	.handler	= sysrq_reboot_handler,
>  	.help_msg       = "reboot(b)",
>  	.action_msg     = "Resetting",
>  	.enable_mask    = SYSRQ_ENABLE_BOOT,
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/vmalloc: track which page-table levels were modified
  2020-06-09 14:15               ` Guenter Roeck
  2020-06-09 16:23                 ` Joerg Roedel
@ 2020-06-10  7:59                 ` Mike Rapoport
  1 sibling, 0 replies; 14+ messages in thread
From: Mike Rapoport @ 2020-06-10  7:59 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Joerg Roedel, Linus Torvalds, Andrew Morton, Andy Lutomirski,
	Peter Zijlstra, Linux Kernel Mailing List, Guan Xuetao

On Tue, Jun 09, 2020 at 07:15:42AM -0700, Guenter Roeck wrote:
> On 6/9/20 5:10 AM, Joerg Roedel wrote:
> > 
> I have stopped building unicore32 images since v4.19 since there is
> no available compiler that is still supported by the kernel. I am
> surprised that support for it has not been removed from the kernel.

Well, I've done the patches [1] and I'll send them after -rc1 is out.
It'll also give kbuild some time to verify I didn't remove too much :)

[1] https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git/log/?h=rm-unicore

> Guenter

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-10  7:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 23:23 [PATCH] mm/vmalloc: track which page-table levels were modified Guenter Roeck
2020-06-04  8:35 ` Joerg Roedel
2020-06-04 17:16   ` Linus Torvalds
2020-06-04 21:06     ` Andrew Morton
2020-06-04 21:12       ` Linus Torvalds
2020-06-05  8:16         ` Joerg Roedel
2020-06-05 10:00           ` Mike Rapoport
2020-06-09 12:10             ` Joerg Roedel
2020-06-09 14:15               ` Guenter Roeck
2020-06-09 16:23                 ` Joerg Roedel
2020-06-09 17:07                   ` Guenter Roeck
2020-06-10  7:59                 ` Mike Rapoport
2020-06-09 15:24               ` Mike Rapoport
2020-06-05  8:12     ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).