All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm, vmalloc: properly track vmalloc users
@ 2017-05-02 13:46 ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-02 13:46 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Vlastimil Babka, linux-mm, LKML, Michal Hocko

From: Michal Hocko <mhocko@suse.com>

__vmalloc_node_flags used to be static inline but this has changed by
"mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
it as well and the code is outside of the vmalloc proper. I haven't
realized that changing this will lead to a subtle bug though. The
function is responsible to track the caller as well. This caller is
then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
then we would get only direct users of __vmalloc_node_flags as callers
(e.g. v[mz]alloc) which reduces usefulness of this debugging feature
considerably. It simply doesn't help to see that the given range belongs
to vmalloc as a caller:
0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3

We really want to catch the _caller_ of the vmalloc function. Fix this
issue by making __vmalloc_node_flags static inline again.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Hi Andrew,
this is a follow up fix for mm-introduce-kvalloc-helpers.patch currently
sitting in your mmotm tree. You can either fold this into it or just
keep it on its own which I would consider slightly better for the
reference.  Anyway it would be great if it could hit the Linus' tree
along with the original patch.

I am sorry, I should have noticed this earlier.

 include/linux/vmalloc.h | 19 +++++++++++++++++++
 mm/vmalloc.c            | 12 +-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 46991ad3ddd5..0328ce003992 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -6,6 +6,7 @@
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <asm/page.h>		/* pgprot_t */
+#include <asm/pgtable.h>	/* PAGE_KERNEL */
 #include <linux/rbtree.h>
 
 struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
@@ -80,7 +81,25 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
 			unsigned long start, unsigned long end, gfp_t gfp_mask,
 			pgprot_t prot, unsigned long vm_flags, int node,
 			const void *caller);
+#ifndef CONFIG_MMU
 extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
+#else
+extern void *__vmalloc_node(unsigned long size, unsigned long align,
+			    gfp_t gfp_mask, pgprot_t prot,
+			    int node, const void *caller);
+
+/*
+ * We really want to have this inlined due to caller tracking. This
+ * function is used by the highlevel vmalloc apis and so we want to track
+ * their callers and inlining will achieve that.
+ */
+static inline void *__vmalloc_node_flags(unsigned long size,
+					int node, gfp_t flags)
+{
+	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
+					node, __builtin_return_address(0));
+}
+#endif
 
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 65912eb93a2c..201eb20ba96a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1649,9 +1649,6 @@ void *vmap(struct page **pages, unsigned int count,
 }
 EXPORT_SYMBOL(vmap);
 
-static void *__vmalloc_node(unsigned long size, unsigned long align,
-			    gfp_t gfp_mask, pgprot_t prot,
-			    int node, const void *caller);
 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 				 pgprot_t prot, int node)
 {
@@ -1794,7 +1791,7 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
  *	with mm people.
  *
  */
-static void *__vmalloc_node(unsigned long size, unsigned long align,
+void *__vmalloc_node(unsigned long size, unsigned long align,
 			    gfp_t gfp_mask, pgprot_t prot,
 			    int node, const void *caller)
 {
@@ -1809,13 +1806,6 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
 }
 EXPORT_SYMBOL(__vmalloc);
 
-void *__vmalloc_node_flags(unsigned long size,
-					int node, gfp_t flags)
-{
-	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
-					node, __builtin_return_address(0));
-}
-
 /**
  *	vmalloc  -  allocate virtually contiguous memory
  *	@size:		allocation size
-- 
2.11.0

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

* [PATCH] mm, vmalloc: properly track vmalloc users
@ 2017-05-02 13:46 ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-02 13:46 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Vlastimil Babka, linux-mm, LKML, Michal Hocko

From: Michal Hocko <mhocko@suse.com>

__vmalloc_node_flags used to be static inline but this has changed by
"mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
it as well and the code is outside of the vmalloc proper. I haven't
realized that changing this will lead to a subtle bug though. The
function is responsible to track the caller as well. This caller is
then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
then we would get only direct users of __vmalloc_node_flags as callers
(e.g. v[mz]alloc) which reduces usefulness of this debugging feature
considerably. It simply doesn't help to see that the given range belongs
to vmalloc as a caller:
0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3

We really want to catch the _caller_ of the vmalloc function. Fix this
issue by making __vmalloc_node_flags static inline again.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Hi Andrew,
this is a follow up fix for mm-introduce-kvalloc-helpers.patch currently
sitting in your mmotm tree. You can either fold this into it or just
keep it on its own which I would consider slightly better for the
reference.  Anyway it would be great if it could hit the Linus' tree
along with the original patch.

I am sorry, I should have noticed this earlier.

 include/linux/vmalloc.h | 19 +++++++++++++++++++
 mm/vmalloc.c            | 12 +-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 46991ad3ddd5..0328ce003992 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -6,6 +6,7 @@
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <asm/page.h>		/* pgprot_t */
+#include <asm/pgtable.h>	/* PAGE_KERNEL */
 #include <linux/rbtree.h>
 
 struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
@@ -80,7 +81,25 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
 			unsigned long start, unsigned long end, gfp_t gfp_mask,
 			pgprot_t prot, unsigned long vm_flags, int node,
 			const void *caller);
+#ifndef CONFIG_MMU
 extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
+#else
+extern void *__vmalloc_node(unsigned long size, unsigned long align,
+			    gfp_t gfp_mask, pgprot_t prot,
+			    int node, const void *caller);
+
+/*
+ * We really want to have this inlined due to caller tracking. This
+ * function is used by the highlevel vmalloc apis and so we want to track
+ * their callers and inlining will achieve that.
+ */
+static inline void *__vmalloc_node_flags(unsigned long size,
+					int node, gfp_t flags)
+{
+	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
+					node, __builtin_return_address(0));
+}
+#endif
 
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 65912eb93a2c..201eb20ba96a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1649,9 +1649,6 @@ void *vmap(struct page **pages, unsigned int count,
 }
 EXPORT_SYMBOL(vmap);
 
-static void *__vmalloc_node(unsigned long size, unsigned long align,
-			    gfp_t gfp_mask, pgprot_t prot,
-			    int node, const void *caller);
 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 				 pgprot_t prot, int node)
 {
@@ -1794,7 +1791,7 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
  *	with mm people.
  *
  */
-static void *__vmalloc_node(unsigned long size, unsigned long align,
+void *__vmalloc_node(unsigned long size, unsigned long align,
 			    gfp_t gfp_mask, pgprot_t prot,
 			    int node, const void *caller)
 {
@@ -1809,13 +1806,6 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
 }
 EXPORT_SYMBOL(__vmalloc);
 
-void *__vmalloc_node_flags(unsigned long size,
-					int node, gfp_t flags)
-{
-	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
-					node, __builtin_return_address(0));
-}
-
 /**
  *	vmalloc  -  allocate virtually contiguous memory
  *	@size:		allocation size
-- 
2.11.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
  2017-05-02 13:46 ` Michal Hocko
  (?)
@ 2017-05-03  0:52 ` kbuild test robot
  2017-05-03  6:37     ` Michal Hocko
  -1 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2017-05-03  0:52 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML, Michal Hocko

[-- Attachment #1: Type: text/plain, Size: 5669 bytes --]

Hi Michal,

[auto build test ERROR on mmotm/master]
[also build test ERROR on next-20170502]
[cannot apply to v4.11]
[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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: m68k-m5475evb_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

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

   In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
                    from arch/m68k/include/asm/pgtable.h:4,
                    from include/linux/vmalloc.h:9,
                    from arch/m68k/kernel/module.c:9:
   arch/m68k/include/asm/mcf_pgtable.h: In function 'nocache_page':
>> arch/m68k/include/asm/mcf_pgtable.h:339:43: error: 'init_mm' undeclared (first use in this function)
    #define pgd_offset_k(address) pgd_offset(&init_mm, address)
                                              ^
   arch/m68k/include/asm/mcf_pgtable.h:334:35: note: in definition of macro 'pgd_offset'
    #define pgd_offset(mm, address) ((mm)->pgd + pgd_index(address))
                                      ^
>> arch/m68k/include/asm/mcf_pgtable.h:366:8: note: in expansion of macro 'pgd_offset_k'
     dir = pgd_offset_k(addr);
           ^
   arch/m68k/include/asm/mcf_pgtable.h:339:43: note: each undeclared identifier is reported only once for each function it appears in
    #define pgd_offset_k(address) pgd_offset(&init_mm, address)
                                              ^
   arch/m68k/include/asm/mcf_pgtable.h:334:35: note: in definition of macro 'pgd_offset'
    #define pgd_offset(mm, address) ((mm)->pgd + pgd_index(address))
                                      ^
>> arch/m68k/include/asm/mcf_pgtable.h:366:8: note: in expansion of macro 'pgd_offset_k'
     dir = pgd_offset_k(addr);
           ^
   arch/m68k/include/asm/mcf_pgtable.h: In function 'cache_page':
>> arch/m68k/include/asm/mcf_pgtable.h:339:43: error: 'init_mm' undeclared (first use in this function)
    #define pgd_offset_k(address) pgd_offset(&init_mm, address)
                                              ^
   arch/m68k/include/asm/mcf_pgtable.h:334:35: note: in definition of macro 'pgd_offset'
    #define pgd_offset(mm, address) ((mm)->pgd + pgd_index(address))
                                      ^
   arch/m68k/include/asm/mcf_pgtable.h:382:8: note: in expansion of macro 'pgd_offset_k'
     dir = pgd_offset_k(addr);
           ^

vim +/init_mm +339 arch/m68k/include/asm/mcf_pgtable.h

91521c2e Greg Ungerer 2011-10-14  333  #define pgd_index(address)	((address) >> PGDIR_SHIFT)
91521c2e Greg Ungerer 2011-10-14  334  #define pgd_offset(mm, address)	((mm)->pgd + pgd_index(address))
91521c2e Greg Ungerer 2011-10-14  335  
91521c2e Greg Ungerer 2011-10-14  336  /*
91521c2e Greg Ungerer 2011-10-14  337   * Find an entry in a kernel pagetable directory.
91521c2e Greg Ungerer 2011-10-14  338   */
91521c2e Greg Ungerer 2011-10-14 @339  #define pgd_offset_k(address)	pgd_offset(&init_mm, address)
91521c2e Greg Ungerer 2011-10-14  340  
91521c2e Greg Ungerer 2011-10-14  341  /*
91521c2e Greg Ungerer 2011-10-14  342   * Find an entry in the second-level pagetable.
91521c2e Greg Ungerer 2011-10-14  343   */
91521c2e Greg Ungerer 2011-10-14  344  static inline pmd_t *pmd_offset(pgd_t *pgd, unsigned long address)
91521c2e Greg Ungerer 2011-10-14  345  {
91521c2e Greg Ungerer 2011-10-14  346  	return (pmd_t *) pgd;
91521c2e Greg Ungerer 2011-10-14  347  }
91521c2e Greg Ungerer 2011-10-14  348  
91521c2e Greg Ungerer 2011-10-14  349  /*
91521c2e Greg Ungerer 2011-10-14  350   * Find an entry in the third-level pagetable.
91521c2e Greg Ungerer 2011-10-14  351   */
91521c2e Greg Ungerer 2011-10-14  352  #define __pte_offset(address)	((address >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
91521c2e Greg Ungerer 2011-10-14  353  #define pte_offset_kernel(dir, address) \
91521c2e Greg Ungerer 2011-10-14  354  	((pte_t *) __pmd_page(*(dir)) + __pte_offset(address))
91521c2e Greg Ungerer 2011-10-14  355  
91521c2e Greg Ungerer 2011-10-14  356  /*
91521c2e Greg Ungerer 2011-10-14  357   * Disable caching for page at given kernel virtual address.
91521c2e Greg Ungerer 2011-10-14  358   */
91521c2e Greg Ungerer 2011-10-14  359  static inline void nocache_page(void *vaddr)
91521c2e Greg Ungerer 2011-10-14  360  {
91521c2e Greg Ungerer 2011-10-14  361  	pgd_t *dir;
91521c2e Greg Ungerer 2011-10-14  362  	pmd_t *pmdp;
91521c2e Greg Ungerer 2011-10-14  363  	pte_t *ptep;
91521c2e Greg Ungerer 2011-10-14  364  	unsigned long addr = (unsigned long) vaddr;
91521c2e Greg Ungerer 2011-10-14  365  
91521c2e Greg Ungerer 2011-10-14 @366  	dir = pgd_offset_k(addr);
91521c2e Greg Ungerer 2011-10-14  367  	pmdp = pmd_offset(dir, addr);
91521c2e Greg Ungerer 2011-10-14  368  	ptep = pte_offset_kernel(pmdp, addr);
91521c2e Greg Ungerer 2011-10-14  369  	*ptep = pte_mknocache(*ptep);

:::::: The code at line 339 was first introduced by commit
:::::: 91521c2ea6e3d5a790df40988101ad099ddbf7c8 m68k: page table support definitions and code for ColdFire MMU

:::::: TO: Greg Ungerer <gerg@uclinux.org>
:::::: CC: Greg Ungerer <gerg@uclinux.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6516 bytes --]

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
  2017-05-02 13:46 ` Michal Hocko
  (?)
  (?)
@ 2017-05-03  1:29 ` kbuild test robot
  -1 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2017-05-03  1:29 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML, Michal Hocko

[-- Attachment #1: Type: text/plain, Size: 4363 bytes --]

Hi Michal,

[auto build test ERROR on mmotm/master]
[also build test ERROR on next-20170502]
[cannot apply to v4.11]
[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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: m68k-multi_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   In file included from arch/m68k/include/asm/pgtable_mm.h:147:0,
                    from arch/m68k/include/asm/pgtable.h:4,
                    from include/linux/vmalloc.h:9,
                    from fs/nfsd/nfscache.c:12:
   arch/m68k/include/asm/motorola_pgtable.h: In function 'pgd_offset':
>> arch/m68k/include/asm/motorola_pgtable.h:198:11: error: dereferencing pointer to incomplete type
     return mm->pgd + pgd_index(address);
              ^

vim +198 arch/m68k/include/asm/motorola_pgtable.h

^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  182  }
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  183  static inline pte_t pte_mkcache(pte_t pte)
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  184  {
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  185  	pte_val(pte) = (pte_val(pte) & _CACHEMASK040) | m68k_supervisor_cachemode;
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  186  	return pte;
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  187  }
7e675137 include/asm-m68k/motorola_pgtable.h Nick Piggin        2008-04-28  188  static inline pte_t pte_mkspecial(pte_t pte)	{ return pte; }
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  189  
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  190  #define PAGE_DIR_OFFSET(tsk,address) pgd_offset((tsk),(address))
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  191  
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  192  #define pgd_index(address)     ((address) >> PGDIR_SHIFT)
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  193  
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  194  /* to find an entry in a page-table-directory */
5b808a59 include/asm-m68k/motorola_pgtable.h Geert Uytterhoeven 2008-02-07  195  static inline pgd_t *pgd_offset(const struct mm_struct *mm,
5b808a59 include/asm-m68k/motorola_pgtable.h Geert Uytterhoeven 2008-02-07  196  				unsigned long address)
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  197  {
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16 @198  	return mm->pgd + pgd_index(address);
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  199  }
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  200  
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  201  #define swapper_pg_dir kernel_pg_dir
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  202  extern pgd_t kernel_pg_dir[128];
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  203  
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  204  static inline pgd_t *pgd_offset_k(unsigned long address)
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  205  {
^1da177e include/asm-m68k/motorola_pgtable.h Linus Torvalds     2005-04-16  206  	return kernel_pg_dir + (address >> PGDIR_SHIFT);

:::::: The code at line 198 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13571 bytes --]

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
  2017-05-03  0:52 ` kbuild test robot
@ 2017-05-03  6:37     ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-03  6:37 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:52:01, kbuild test robot wrote:
> Hi Michal,
> 
> [auto build test ERROR on mmotm/master]
> [also build test ERROR on next-20170502]
> [cannot apply to v4.11]
> [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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
> base:   git://git.cmpxchg.org/linux-mmotm.git master
> config: m68k-m5475evb_defconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 4.9.0
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=m68k 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
>                     from arch/m68k/include/asm/pgtable.h:4,
>                     from include/linux/vmalloc.h:9,
>                     from arch/m68k/kernel/module.c:9:

OK, I was little bit worried to pull pgtable.h include in, but my cross
compile build test battery didn't show any issues. I do not have m68k
there though. So let's just do this differently. The following updated
patch hasn't passed the full build test battery but it should just work.

Thanks for your testing.
---
>From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Tue, 2 May 2017 11:18:29 +0200
Subject: [PATCH] mm, vmalloc: properly track vmalloc users

__vmalloc_node_flags used to be static inline but this has changed by
"mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
it as well and the code is outside of the vmalloc proper. I haven't
realized that changing this will lead to a subtle bug though. The
function is responsible to track the caller as well. This caller is
then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
then we would get only direct users of __vmalloc_node_flags as callers
(e.g. v[mz]alloc) which reduces usefulness of this debugging feature
considerably. It simply doesn't help to see that the given range belongs
to vmalloc as a caller:
0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3

We really want to catch the _caller_ of the vmalloc function. Fix this
issue by making __vmalloc_node_flags static inline again and export
__vmalloc_node_flags_caller for kvmalloc_node().

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/vmalloc.h | 16 +++++++++++++++-
 mm/util.c               |  3 ++-
 mm/vmalloc.c            |  8 +++++++-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 46991ad3ddd5..4a0fabeb1e92 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
 			unsigned long start, unsigned long end, gfp_t gfp_mask,
 			pgprot_t prot, unsigned long vm_flags, int node,
 			const void *caller);
-extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
+#ifndef CONFIG_MMU
+extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
+static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
+{
+	return __vmalloc_node_flags(size, node, flags);
+}
+#else
+/*
+ * We really want to have this inlined due to caller tracking. This
+ * function is used by the highlevel vmalloc apis and so we want to track
+ * their callers and inlining will achieve that.
+ */
+extern void *__vmalloc_node_flags_caller(unsigned long size,
+					int node, gfp_t flags, void* caller);
+#endif
 
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
diff --git a/mm/util.c b/mm/util.c
index 3022051da938..c35e5870921d 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
 	if (ret || size <= PAGE_SIZE)
 		return ret;
 
-	return __vmalloc_node_flags(size, node, flags);
+	return __vmalloc_node_flags_caller(size, node, flags,
+			__builtin_return_address(0));
 }
 EXPORT_SYMBOL(kvmalloc_node);
 
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 65912eb93a2c..1a97d4a31406 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
 }
 EXPORT_SYMBOL(__vmalloc);
 
-void *__vmalloc_node_flags(unsigned long size,
+static inline void *__vmalloc_node_flags(unsigned long size,
 					int node, gfp_t flags)
 {
 	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
 					node, __builtin_return_address(0));
 }
 
+
+void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
+{
+	return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
+}
+
 /**
  *	vmalloc  -  allocate virtually contiguous memory
  *	@size:		allocation size
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
@ 2017-05-03  6:37     ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-03  6:37 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:52:01, kbuild test robot wrote:
> Hi Michal,
> 
> [auto build test ERROR on mmotm/master]
> [also build test ERROR on next-20170502]
> [cannot apply to v4.11]
> [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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
> base:   git://git.cmpxchg.org/linux-mmotm.git master
> config: m68k-m5475evb_defconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 4.9.0
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=m68k 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
>                     from arch/m68k/include/asm/pgtable.h:4,
>                     from include/linux/vmalloc.h:9,
>                     from arch/m68k/kernel/module.c:9:

OK, I was little bit worried to pull pgtable.h include in, but my cross
compile build test battery didn't show any issues. I do not have m68k
there though. So let's just do this differently. The following updated
patch hasn't passed the full build test battery but it should just work.

Thanks for your testing.
---

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
  2017-05-03  6:37     ` Michal Hocko
@ 2017-05-03 13:09       ` Michal Hocko
  -1 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-03 13:09 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:37:50, Michal Hocko wrote:
> On Wed 03-05-17 08:52:01, kbuild test robot wrote:
> > Hi Michal,
> > 
> > [auto build test ERROR on mmotm/master]
> > [also build test ERROR on next-20170502]
> > [cannot apply to v4.11]
> > [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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
> > base:   git://git.cmpxchg.org/linux-mmotm.git master
> > config: m68k-m5475evb_defconfig (attached as .config)
> > compiler: m68k-linux-gcc (GCC) 4.9.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=m68k 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >    In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
> >                     from arch/m68k/include/asm/pgtable.h:4,
> >                     from include/linux/vmalloc.h:9,
> >                     from arch/m68k/kernel/module.c:9:
> 
> OK, I was little bit worried to pull pgtable.h include in, but my cross
> compile build test battery didn't show any issues. I do not have m68k
> there though. So let's just do this differently. The following updated
> patch hasn't passed the full build test battery but it should just work.

I assume that the silence from the kbuild robot means good to go here.
Andrew, could you replace the previous patch by the following one please?
 
> From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 2 May 2017 11:18:29 +0200
> Subject: [PATCH] mm, vmalloc: properly track vmalloc users
> 
> __vmalloc_node_flags used to be static inline but this has changed by
> "mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
> it as well and the code is outside of the vmalloc proper. I haven't
> realized that changing this will lead to a subtle bug though. The
> function is responsible to track the caller as well. This caller is
> then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
> then we would get only direct users of __vmalloc_node_flags as callers
> (e.g. v[mz]alloc) which reduces usefulness of this debugging feature
> considerably. It simply doesn't help to see that the given range belongs
> to vmalloc as a caller:
> 0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
> 0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 
> We really want to catch the _caller_ of the vmalloc function. Fix this
> issue by making __vmalloc_node_flags static inline again and export
> __vmalloc_node_flags_caller for kvmalloc_node().
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/vmalloc.h | 16 +++++++++++++++-
>  mm/util.c               |  3 ++-
>  mm/vmalloc.c            |  8 +++++++-
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad3ddd5..4a0fabeb1e92 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  			unsigned long start, unsigned long end, gfp_t gfp_mask,
>  			pgprot_t prot, unsigned long vm_flags, int node,
>  			const void *caller);
> -extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
> +#ifndef CONFIG_MMU
> +extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
> +static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
> +{
> +	return __vmalloc_node_flags(size, node, flags);
> +}
> +#else
> +/*
> + * We really want to have this inlined due to caller tracking. This
> + * function is used by the highlevel vmalloc apis and so we want to track
> + * their callers and inlining will achieve that.
> + */
> +extern void *__vmalloc_node_flags_caller(unsigned long size,
> +					int node, gfp_t flags, void* caller);
> +#endif
>  
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
> diff --git a/mm/util.c b/mm/util.c
> index 3022051da938..c35e5870921d 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>  	if (ret || size <= PAGE_SIZE)
>  		return ret;
>  
> -	return __vmalloc_node_flags(size, node, flags);
> +	return __vmalloc_node_flags_caller(size, node, flags,
> +			__builtin_return_address(0));
>  }
>  EXPORT_SYMBOL(kvmalloc_node);
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 65912eb93a2c..1a97d4a31406 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>  
> -void *__vmalloc_node_flags(unsigned long size,
> +static inline void *__vmalloc_node_flags(unsigned long size,
>  					int node, gfp_t flags)
>  {
>  	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
>  					node, __builtin_return_address(0));
>  }
>  
> +
> +void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
> +{
> +	return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
> +}
> +
>  /**
>   *	vmalloc  -  allocate virtually contiguous memory
>   *	@size:		allocation size
> -- 
> 2.11.0
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
@ 2017-05-03 13:09       ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-03 13:09 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Andrew Morton, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:37:50, Michal Hocko wrote:
> On Wed 03-05-17 08:52:01, kbuild test robot wrote:
> > Hi Michal,
> > 
> > [auto build test ERROR on mmotm/master]
> > [also build test ERROR on next-20170502]
> > [cannot apply to v4.11]
> > [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/Michal-Hocko/mm-vmalloc-properly-track-vmalloc-users/20170503-065022
> > base:   git://git.cmpxchg.org/linux-mmotm.git master
> > config: m68k-m5475evb_defconfig (attached as .config)
> > compiler: m68k-linux-gcc (GCC) 4.9.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=m68k 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >    In file included from arch/m68k/include/asm/pgtable_mm.h:145:0,
> >                     from arch/m68k/include/asm/pgtable.h:4,
> >                     from include/linux/vmalloc.h:9,
> >                     from arch/m68k/kernel/module.c:9:
> 
> OK, I was little bit worried to pull pgtable.h include in, but my cross
> compile build test battery didn't show any issues. I do not have m68k
> there though. So let's just do this differently. The following updated
> patch hasn't passed the full build test battery but it should just work.

I assume that the silence from the kbuild robot means good to go here.
Andrew, could you replace the previous patch by the following one please?
 
> From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 2 May 2017 11:18:29 +0200
> Subject: [PATCH] mm, vmalloc: properly track vmalloc users
> 
> __vmalloc_node_flags used to be static inline but this has changed by
> "mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
> it as well and the code is outside of the vmalloc proper. I haven't
> realized that changing this will lead to a subtle bug though. The
> function is responsible to track the caller as well. This caller is
> then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
> then we would get only direct users of __vmalloc_node_flags as callers
> (e.g. v[mz]alloc) which reduces usefulness of this debugging feature
> considerably. It simply doesn't help to see that the given range belongs
> to vmalloc as a caller:
> 0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
> 0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 
> We really want to catch the _caller_ of the vmalloc function. Fix this
> issue by making __vmalloc_node_flags static inline again and export
> __vmalloc_node_flags_caller for kvmalloc_node().
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/vmalloc.h | 16 +++++++++++++++-
>  mm/util.c               |  3 ++-
>  mm/vmalloc.c            |  8 +++++++-
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad3ddd5..4a0fabeb1e92 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  			unsigned long start, unsigned long end, gfp_t gfp_mask,
>  			pgprot_t prot, unsigned long vm_flags, int node,
>  			const void *caller);
> -extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
> +#ifndef CONFIG_MMU
> +extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
> +static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
> +{
> +	return __vmalloc_node_flags(size, node, flags);
> +}
> +#else
> +/*
> + * We really want to have this inlined due to caller tracking. This
> + * function is used by the highlevel vmalloc apis and so we want to track
> + * their callers and inlining will achieve that.
> + */
> +extern void *__vmalloc_node_flags_caller(unsigned long size,
> +					int node, gfp_t flags, void* caller);
> +#endif
>  
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
> diff --git a/mm/util.c b/mm/util.c
> index 3022051da938..c35e5870921d 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>  	if (ret || size <= PAGE_SIZE)
>  		return ret;
>  
> -	return __vmalloc_node_flags(size, node, flags);
> +	return __vmalloc_node_flags_caller(size, node, flags,
> +			__builtin_return_address(0));
>  }
>  EXPORT_SYMBOL(kvmalloc_node);
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 65912eb93a2c..1a97d4a31406 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>  
> -void *__vmalloc_node_flags(unsigned long size,
> +static inline void *__vmalloc_node_flags(unsigned long size,
>  					int node, gfp_t flags)
>  {
>  	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
>  					node, __builtin_return_address(0));
>  }
>  
> +
> +void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
> +{
> +	return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
> +}
> +
>  /**
>   *	vmalloc  -  allocate virtually contiguous memory
>   *	@size:		allocation size
> -- 
> 2.11.0
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
  2017-05-03  6:37     ` Michal Hocko
@ 2017-05-09 11:19       ` Michal Hocko
  -1 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-09 11:19 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kbuild-all, kbuild test robot, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:37:50, Michal Hocko wrote:
> From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 2 May 2017 11:18:29 +0200
> Subject: [PATCH] mm, vmalloc: properly track vmalloc users
> 
> __vmalloc_node_flags used to be static inline but this has changed by
> "mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
> it as well and the code is outside of the vmalloc proper. I haven't
> realized that changing this will lead to a subtle bug though. The
> function is responsible to track the caller as well. This caller is
> then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
> then we would get only direct users of __vmalloc_node_flags as callers
> (e.g. v[mz]alloc) which reduces usefulness of this debugging feature
> considerably. It simply doesn't help to see that the given range belongs
> to vmalloc as a caller:
> 0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
> 0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 
> We really want to catch the _caller_ of the vmalloc function. Fix this
> issue by making __vmalloc_node_flags static inline again and export
> __vmalloc_node_flags_caller for kvmalloc_node().
> 

The "mm: introduce kv[mz]alloc helpers" got merged in the mean time so I
believe the patch should mention

Fixes: a7c3e901a46f ("mm: introduce kv[mz]alloc helpers")

> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/vmalloc.h | 16 +++++++++++++++-
>  mm/util.c               |  3 ++-
>  mm/vmalloc.c            |  8 +++++++-
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad3ddd5..4a0fabeb1e92 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  			unsigned long start, unsigned long end, gfp_t gfp_mask,
>  			pgprot_t prot, unsigned long vm_flags, int node,
>  			const void *caller);
> -extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
> +#ifndef CONFIG_MMU
> +extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
> +static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
> +{
> +	return __vmalloc_node_flags(size, node, flags);
> +}
> +#else
> +/*
> + * We really want to have this inlined due to caller tracking. This
> + * function is used by the highlevel vmalloc apis and so we want to track
> + * their callers and inlining will achieve that.
> + */
> +extern void *__vmalloc_node_flags_caller(unsigned long size,
> +					int node, gfp_t flags, void* caller);
> +#endif
>  
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
> diff --git a/mm/util.c b/mm/util.c
> index 3022051da938..c35e5870921d 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>  	if (ret || size <= PAGE_SIZE)
>  		return ret;
>  
> -	return __vmalloc_node_flags(size, node, flags);
> +	return __vmalloc_node_flags_caller(size, node, flags,
> +			__builtin_return_address(0));
>  }
>  EXPORT_SYMBOL(kvmalloc_node);
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 65912eb93a2c..1a97d4a31406 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>  
> -void *__vmalloc_node_flags(unsigned long size,
> +static inline void *__vmalloc_node_flags(unsigned long size,
>  					int node, gfp_t flags)
>  {
>  	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
>  					node, __builtin_return_address(0));
>  }
>  
> +
> +void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
> +{
> +	return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
> +}
> +
>  /**
>   *	vmalloc  -  allocate virtually contiguous memory
>   *	@size:		allocation size
> -- 
> 2.11.0
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] mm, vmalloc: properly track vmalloc users
@ 2017-05-09 11:19       ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2017-05-09 11:19 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kbuild-all, kbuild test robot, Vlastimil Babka, linux-mm, LKML

On Wed 03-05-17 08:37:50, Michal Hocko wrote:
> From 33a6239135cb444654f48d5e942e7f34898e24ea Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 2 May 2017 11:18:29 +0200
> Subject: [PATCH] mm, vmalloc: properly track vmalloc users
> 
> __vmalloc_node_flags used to be static inline but this has changed by
> "mm: introduce kv[mz]alloc helpers" because kvmalloc_node needs to use
> it as well and the code is outside of the vmalloc proper. I haven't
> realized that changing this will lead to a subtle bug though. The
> function is responsible to track the caller as well. This caller is
> then printed by /proc/vmallocinfo. If __vmalloc_node_flags is not inline
> then we would get only direct users of __vmalloc_node_flags as callers
> (e.g. v[mz]alloc) which reduces usefulness of this debugging feature
> considerably. It simply doesn't help to see that the given range belongs
> to vmalloc as a caller:
> 0xffffc90002c79000-0xffffc90002c7d000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N0=3
> 0xffffc90002c81000-0xffffc90002c85000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c8d000-0xffffc90002c91000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 0xffffc90002c95000-0xffffc90002c99000   16384 vmalloc+0x16/0x18 pages=3 vmalloc N1=3
> 
> We really want to catch the _caller_ of the vmalloc function. Fix this
> issue by making __vmalloc_node_flags static inline again and export
> __vmalloc_node_flags_caller for kvmalloc_node().
> 

The "mm: introduce kv[mz]alloc helpers" got merged in the mean time so I
believe the patch should mention

Fixes: a7c3e901a46f ("mm: introduce kv[mz]alloc helpers")

> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/vmalloc.h | 16 +++++++++++++++-
>  mm/util.c               |  3 ++-
>  mm/vmalloc.c            |  8 +++++++-
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 46991ad3ddd5..4a0fabeb1e92 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -80,7 +80,21 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  			unsigned long start, unsigned long end, gfp_t gfp_mask,
>  			pgprot_t prot, unsigned long vm_flags, int node,
>  			const void *caller);
> -extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
> +#ifndef CONFIG_MMU
> +extern void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags);
> +static inline void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void* caller)
> +{
> +	return __vmalloc_node_flags(size, node, flags);
> +}
> +#else
> +/*
> + * We really want to have this inlined due to caller tracking. This
> + * function is used by the highlevel vmalloc apis and so we want to track
> + * their callers and inlining will achieve that.
> + */
> +extern void *__vmalloc_node_flags_caller(unsigned long size,
> +					int node, gfp_t flags, void* caller);
> +#endif
>  
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
> diff --git a/mm/util.c b/mm/util.c
> index 3022051da938..c35e5870921d 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -380,7 +380,8 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>  	if (ret || size <= PAGE_SIZE)
>  		return ret;
>  
> -	return __vmalloc_node_flags(size, node, flags);
> +	return __vmalloc_node_flags_caller(size, node, flags,
> +			__builtin_return_address(0));
>  }
>  EXPORT_SYMBOL(kvmalloc_node);
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 65912eb93a2c..1a97d4a31406 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1809,13 +1809,19 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>  
> -void *__vmalloc_node_flags(unsigned long size,
> +static inline void *__vmalloc_node_flags(unsigned long size,
>  					int node, gfp_t flags)
>  {
>  	return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
>  					node, __builtin_return_address(0));
>  }
>  
> +
> +void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags, void *caller)
> +{
> +	return __vmalloc_node(size, 1, flags, PAGE_KERNEL, node, caller);
> +}
> +
>  /**
>   *	vmalloc  -  allocate virtually contiguous memory
>   *	@size:		allocation size
> -- 
> 2.11.0
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

end of thread, other threads:[~2017-05-09 11:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-02 13:46 [PATCH] mm, vmalloc: properly track vmalloc users Michal Hocko
2017-05-02 13:46 ` Michal Hocko
2017-05-03  0:52 ` kbuild test robot
2017-05-03  6:37   ` Michal Hocko
2017-05-03  6:37     ` Michal Hocko
2017-05-03 13:09     ` Michal Hocko
2017-05-03 13:09       ` Michal Hocko
2017-05-09 11:19     ` Michal Hocko
2017-05-09 11:19       ` Michal Hocko
2017-05-03  1:29 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.