| Message ID | 1471360856-16916-1-git-send-email-catalin.marinas@arm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 17, 2016 at 01:15:53AM +0800, kbuild test robot wrote: > [auto build test ERROR on mmotm/master] > [also build test ERROR on v4.8-rc2 next-20160816] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Catalin-Marinas/mm-kmemleak-Avoid-using-__va-on-addresses-that-don-t-have-a-lowmem-mapping/20160816-232733 > base: git://git.cmpxchg.org/linux-mmotm.git master > config: tile-tilegx_defconfig (attached as .config) > compiler: tilegx-linux-gcc (GCC) 4.6.2 > reproduce: > wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=tile > > All error/warnings (new ones prefixed by >>): > > In file included from include/linux/kmemleak.h:24:0, > from include/linux/slab.h:117, > from arch/tile/include/asm/pgtable.h:27, > from mm/init-mm.c:9: > include/linux/mm.h: In function 'is_vmalloc_addr': > include/linux/mm.h:486:17: error: 'VMALLOC_START' undeclared (first use in this function) > include/linux/mm.h:486:17: note: each undeclared identifier is reported only once for each function it appears in > include/linux/mm.h:486:41: error: 'VMALLOC_END' undeclared (first use in this function) > include/linux/mm.h: In function 'maybe_mkwrite': > include/linux/mm.h:624:3: error: implicit declaration of function 'pte_mkwrite' > include/linux/mm.h:624:7: error: incompatible types when assigning to type 'pte_t' from type 'int' > In file included from include/linux/kmemleak.h:24:0, > from include/linux/slab.h:117, > from arch/tile/include/asm/pgtable.h:27, > from mm/init-mm.c:9: It looks like some architectures don't really like including linux/mm.h from linux/kmemleak.h. I'll change the patch to avoid this include and explicitly declare high_memory in kmemleak.h -- Catalin
On Tue, Aug 16, 2016 at 04:20:56PM +0100, Catalin Marinas wrote: > diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h > index 4894c6888bc6..380f72bc3657 100644 > --- a/include/linux/kmemleak.h > +++ b/include/linux/kmemleak.h > @@ -21,6 +21,7 @@ > #ifndef __KMEMLEAK_H > #define __KMEMLEAK_H > > +#include <linux/mm.h> Given the kbuild-robot reports, this #include doesn't go well on some architectures. > #include <linux/slab.h> > > #ifdef CONFIG_DEBUG_KMEMLEAK > @@ -109,4 +110,29 @@ static inline void kmemleak_no_scan(const void *ptr) > > #endif /* CONFIG_DEBUG_KMEMLEAK */ > > +static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size, > + int min_count, gfp_t gfp) > +{ > + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) > + kmemleak_alloc(__va(phys), size, min_count, gfp); > +} > + > +static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size) > +{ > + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) > + kmemleak_free_part(__va(phys), size); > +} > + > +static inline void kmemleak_not_leak_phys(phys_addr_t phys) > +{ > + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) > + kmemleak_not_leak(__va(phys)); > +} > + > +static inline void kmemleak_ignore_phys(phys_addr_t phys) > +{ > + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) > + kmemleak_ignore(__va(phys)); > +} I'll move these functions out of line and re-post the patch. -- Catalin
On Wed, Aug 17, 2016 at 11:51:41PM +0800, kernel test robot wrote: > FYI, we noticed the following commit: > > https://github.com/0day-ci/linux Catalin-Marinas/mm-kmemleak-Avoid-using-__va-on-addresses-that-don-t-have-a-lowmem-mapping/20160816-232733 > commit 122708b1b91eb3d253baf86a263ead0f1f5cac78 ("mm: kmemleak: Avoid using __va() on addresses that don't have a lowmem mapping") > > in testcase: boot > > on test machine: 1 threads qemu-system-i386 -enable-kvm with 320M memory > > caused below changes: > > +--------------------------------+------------+------------+ > | | 304bec1b1d | 122708b1b9 | > +--------------------------------+------------+------------+ > | boot_successes | 3 | 0 | > | boot_failures | 5 | 8 | > | invoked_oom-killer:gfp_mask=0x | 1 | | > | Mem-Info | 1 | | > | BUG:kernel_test_crashed | 4 | | > | PANIC:early_exception | 0 | 8 | > | EIP_is_at__phys_addr | 0 | 8 | > | BUG:kernel_hang_in_boot_stage | 0 | 2 | > | BUG:kernel_boot_hang | 0 | 6 | > +--------------------------------+------------+------------+ Please disregard this patch. I posted v2 here: http://lkml.kernel.org/g/1471426130-21330-1-git-send-email-catalin.marinas@arm.com (and I'm eager to see the kbuild/kernel test robot results ;)) -- Catalin
On Wed, Aug 17, 2016 at 12:18:08PM -0700, Andrew Morton wrote: > On Wed, 17 Aug 2016 17:10:28 +0100 Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Wed, Aug 17, 2016 at 11:51:41PM +0800, kernel test robot wrote: > > > FYI, we noticed the following commit: > > > > > > https://github.com/0day-ci/linux Catalin-Marinas/mm-kmemleak-Avoid-using-__va-on-addresses-that-don-t-have-a-lowmem-mapping/20160816-232733 > > > commit 122708b1b91eb3d253baf86a263ead0f1f5cac78 ("mm: kmemleak: Avoid using __va() on addresses that don't have a lowmem mapping") > > > > > > in testcase: boot > > > > > > on test machine: 1 threads qemu-system-i386 -enable-kvm with 320M memory > > > > > > caused below changes: > > > > > > +--------------------------------+------------+------------+ > > > | | 304bec1b1d | 122708b1b9 | > > > +--------------------------------+------------+------------+ > > > | boot_successes | 3 | 0 | > > > | boot_failures | 5 | 8 | > > > | invoked_oom-killer:gfp_mask=0x | 1 | | > > > | Mem-Info | 1 | | > > > | BUG:kernel_test_crashed | 4 | | > > > | PANIC:early_exception | 0 | 8 | > > > | EIP_is_at__phys_addr | 0 | 8 | > > > | BUG:kernel_hang_in_boot_stage | 0 | 2 | > > > | BUG:kernel_boot_hang | 0 | 6 | > > > +--------------------------------+------------+------------+ > > > > Please disregard this patch. I posted v2 here: > > > > http://lkml.kernel.org/g/1471426130-21330-1-git-send-email-catalin.marinas@arm.com > > > > (and I'm eager to see the kbuild/kernel test robot results ;)) > > I don't see how the v1->v2 changes could fix a panic? This particular panic is avoided (rather than fixed) in v2 because the config used above has kmemleak disabled, hence there is no __pa(high_memory) call. But you are right, it is likely to trigger once kmemleak is enabled, I think because __pa(high_memory) use isn't valid. I need to reproduce it tomorrow (UK time) but a workaround is to test against __pa(high_memory - 1) or (max_low_pfn << PAGE_SHIFT). -- Catalin
diff --git a/Documentation/kmemleak.txt b/Documentation/kmemleak.txt index 18e24abb3ecf..35e1a8891e3a 100644 --- a/Documentation/kmemleak.txt +++ b/Documentation/kmemleak.txt @@ -155,6 +155,15 @@ kmemleak_erase - erase an old value in a pointer variable kmemleak_alloc_recursive - as kmemleak_alloc but checks the recursiveness kmemleak_free_recursive - as kmemleak_free but checks the recursiveness +The following functions take a physical address as the object pointer +and only perform the corresponding action if the address has a lowmem +mapping: + +kmemleak_alloc_phys +kmemleak_free_part_phys +kmemleak_not_leak_phys +kmemleak_ignore_phys + Dealing with false positives/negatives -------------------------------------- diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h index 4894c6888bc6..380f72bc3657 100644 --- a/include/linux/kmemleak.h +++ b/include/linux/kmemleak.h @@ -21,6 +21,7 @@ #ifndef __KMEMLEAK_H #define __KMEMLEAK_H +#include <linux/mm.h> #include <linux/slab.h> #ifdef CONFIG_DEBUG_KMEMLEAK @@ -109,4 +110,29 @@ static inline void kmemleak_no_scan(const void *ptr) #endif /* CONFIG_DEBUG_KMEMLEAK */ +static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size, + int min_count, gfp_t gfp) +{ + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) + kmemleak_alloc(__va(phys), size, min_count, gfp); +} + +static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size) +{ + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) + kmemleak_free_part(__va(phys), size); +} + +static inline void kmemleak_not_leak_phys(phys_addr_t phys) +{ + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) + kmemleak_not_leak(__va(phys)); +} + +static inline void kmemleak_ignore_phys(phys_addr_t phys) +{ + if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory)) + kmemleak_ignore(__va(phys)); +} + #endif /* __KMEMLEAK_H */ diff --git a/mm/bootmem.c b/mm/bootmem.c index 0aa7dda52402..80f1d70bad2d 100644 --- a/mm/bootmem.c +++ b/mm/bootmem.c @@ -158,7 +158,7 @@ void __init free_bootmem_late(unsigned long physaddr, unsigned long size) { unsigned long cursor, end; - kmemleak_free_part(__va(physaddr), size); + kmemleak_free_part_phys(physaddr, size); cursor = PFN_UP(physaddr); end = PFN_DOWN(physaddr + size); @@ -402,7 +402,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr, { unsigned long start, end; - kmemleak_free_part(__va(physaddr), size); + kmemleak_free_part_phys(physaddr, size); start = PFN_UP(physaddr); end = PFN_DOWN(physaddr + size); @@ -423,7 +423,7 @@ void __init free_bootmem(unsigned long physaddr, unsigned long size) { unsigned long start, end; - kmemleak_free_part(__va(physaddr), size); + kmemleak_free_part_phys(physaddr, size); start = PFN_UP(physaddr); end = PFN_DOWN(physaddr + size); diff --git a/mm/cma.c b/mm/cma.c index bd0e1412475e..384c2cb51b56 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -336,7 +336,7 @@ int __init cma_declare_contiguous(phys_addr_t base, * kmemleak scans/reads tracked objects for pointers to other * objects but this address isn't mapped and accessible */ - kmemleak_ignore(phys_to_virt(addr)); + kmemleak_ignore_phys(addr); base = addr; } diff --git a/mm/memblock.c b/mm/memblock.c index 483197ef613f..30ecea7b45d1 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -723,7 +723,7 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size) (unsigned long long)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + kmemleak_free_part_phys(base, size); return memblock_remove_range(&memblock.reserved, base, size); } @@ -1152,7 +1152,7 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size, * The min_count is set to 0 so that memblock allocations are * never reported as leaks. */ - kmemleak_alloc(__va(found), size, 0, 0); + kmemleak_alloc_phys(found, size, 0, 0); return found; } return 0; @@ -1399,7 +1399,7 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size) memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", __func__, (u64)base, (u64)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + kmemleak_free_part_phys(base, size); memblock_remove_range(&memblock.reserved, base, size); } @@ -1419,7 +1419,7 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size) memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", __func__, (u64)base, (u64)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + kmemleak_free_part_phys(base, size); cursor = PFN_UP(base); end = PFN_DOWN(base + size); diff --git a/mm/nobootmem.c b/mm/nobootmem.c index bd05a70f44b9..a056d31dff7e 100644 --- a/mm/nobootmem.c +++ b/mm/nobootmem.c @@ -81,7 +81,7 @@ void __init free_bootmem_late(unsigned long addr, unsigned long size) { unsigned long cursor, end; - kmemleak_free_part(__va(addr), size); + kmemleak_free_part_phys(addr, size); cursor = PFN_UP(addr); end = PFN_DOWN(addr + size);