Thread (12 messages) 12 messages, 5 authors, 2020-02-11

Re: [PATCH V13] mm/debug: Add tests validating architecture page table helpers

From: Anshuman Khandual <hidden>
Date: 2020-02-11 02:25:57
Also in: linux-alpha, linux-mm, linux-riscv, oe-kbuild-all


On 02/10/2020 04:36 PM, Russell King - ARM Linux admin wrote:
On Mon, Feb 10, 2020 at 11:46:23AM +0100, Christophe Leroy wrote:
quoted

Le 10/02/2020 à 11:02, Russell King - ARM Linux admin a écrit :
quoted
On Mon, Feb 10, 2020 at 07:38:38AM +0100, Christophe Leroy wrote:
quoted

Le 10/02/2020 à 06:35, Anshuman Khandual a écrit :
quoted

On 02/10/2020 10:22 AM, Andrew Morton wrote:
quoted
On Thu, 6 Feb 2020 13:49:35 +0530 Anshuman Khandual [off-list ref] wrote:
quoted
On 02/06/2020 04:40 AM, kbuild test robot wrote:
quoted
Hi Anshuman,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on s390/features linus/master arc/for-next v5.5]
[cannot apply to mmotm/master tip/x86/core arm64/for-next/core next-20200205]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Anshuman-Khandual/mm-debug-Add-tests-validating-architecture-page-table-helpers/20200205-215507
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.5.0
reproduce:
          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
          chmod +x ~/bin/make.cross
          # save the attached .config to linux build tree
          GCC_VERSION=7.5.0 make.cross ARCH=ia64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>

All error/warnings (new ones prefixed by >>):

     In file included from include/asm-generic/pgtable-nopud.h:8:0,
                      from arch/ia64/include/asm/pgtable.h:586,
                      from include/linux/mm.h:99,
                      from include/linux/highmem.h:8,
                      from mm/debug_vm_pgtable.c:14:
     mm/debug_vm_pgtable.c: In function 'pud_clear_tests':
quoted
quoted
include/asm-generic/pgtable-nop4d-hack.h:47:32: error: implicit declaration of function '__pgd'; did you mean '__p4d'? [-Werror=implicit-function-declaration]
      #define __pud(x)    ((pud_t) { __pgd(x) })
                                     ^
quoted
quoted
mm/debug_vm_pgtable.c:141:8: note: in expansion of macro '__pud'
       pud = __pud(pud_val(pud) | RANDOM_ORVALUE);
             ^~~~~
quoted
quoted
include/asm-generic/pgtable-nop4d-hack.h:47:22: warning: missing braces around initializer [-Wmissing-braces]
      #define __pud(x)    ((pud_t) { __pgd(x) })
                           ^
quoted
quoted
mm/debug_vm_pgtable.c:141:8: note: in expansion of macro '__pud'
       pud = __pud(pud_val(pud) | RANDOM_ORVALUE);
             ^~~~~
     cc1: some warnings being treated as errors
This build failure is expected now given that we have allowed DEBUG_VM_PGTABLE
with EXPERT without platform requiring ARCH_HAS_DEBUG_VM_PGTABLE. This problem
i.e build failure caused without a platform __pgd(), is known to exist both on
ia64 and arm (32bit) platforms. Please refer https://lkml.org/lkml/2019/9/24/314
for details where this was discussed earlier.
I'd prefer not to merge a patch which is known to cause build
regressions.  Is there some temporary thing we can do to prevent these
errors until arch maintainers(?) get around to implementing the
long-term fixes?
We could explicitly disable CONFIG_DEBUG_VM_PGTABLE on ia64 and arm platforms
which will ensure that others can still use the EXPERT path.

config DEBUG_VM_PGTABLE
	bool "Debug arch page table for semantics compliance"
	depends on MMU
	depends on !(IA64 || ARM)
	depends on ARCH_HAS_DEBUG_VM_PGTABLE || EXPERT
	default n if !ARCH_HAS_DEBUG_VM_PGTABLE
	default y if DEBUG_VM
On both ia32 and arm, the fix is trivial.

Can we include the fix within this patch, just the same way as the
mm_p4d_folded() fix for x86 ?
Why should arm include a macro for something that nothing (apart from
this checker) requires?  If the checker requires it but the rest of
the kernel does not, it suggests that the checker isn't actually
correct, and the results can't be relied upon.
As far as I can see, the problem is that arm opencodes part of the API
instead of including asm-generic/pgtable-nopmd.h

Here, the ARM has 2 levels, ie only PGD and PTE. But instead of defining
__pgd and __pte and getting everything else from asm-generic, it defines a
__pmd then redefines the folded levels like the pud, etc ...

That's exactly what the checker aims at detecting: architectures than do not
properly use the standard linux page table structures.
There are good reasons for the way ARM does stuff.  The generic crap was
written without regard for the circumstances that ARM has, and thus is
entirely unsuitable for 32-bit ARM.
Since we dont have an agreement here, lets just settle with disabling the
test for now on platforms where the build fails. CONFIG_EXPERT is enabling
this test for better adaptability and coverage, hence how about re framing
the config like this ? This at the least conveys the fact that EXPERT only
works when platform is neither IA64 or ARM.

config DEBUG_VM_PGTABLE
	bool "Debug arch page table for semantics compliance"
	depends on MMU
	depends on ARCH_HAS_DEBUG_VM_PGTABLE || (EXPERT &&  !(IA64 || ARM))
	default n if !ARCH_HAS_DEBUG_VM_PGTABLE
	default y if DEBUG_VM
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help