diff mbox series

mm/slab: Add a __GFP_ACCOUNT GFP flag check for slab allocation

Message ID 20200614063858.85118-1-songmuchun@bytedance.com (mailing list archive)
State New, archived
Headers show
Series mm/slab: Add a __GFP_ACCOUNT GFP flag check for slab allocation | expand

Commit Message

Muchun Song June 14, 2020, 6:38 a.m. UTC 
When a kmem_cache is initialized with SLAB_ACCOUNT slab flag, we must
not call kmem_cache_alloc with __GFP_ACCOUNT GFP flag. In this case,
we can be accounted to kmemcg twice. This is not correct. So we add a
__GFP_ACCOUNT GFP flag check for slab allocation.

We also introduce a new helper named fixup_gfp_flags to do that check.
We can reuse the fixup_gfp_flags for SLAB/SLUB.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 mm/slab.c | 10 +---------
 mm/slab.h | 21 +++++++++++++++++++++
 mm/slub.c | 10 +---------
 3 files changed, 23 insertions(+), 18 deletions(-)

Comments

kernel test robot June 14, 2020, 10:18 a.m. UTC | #1 
Hi Muchun,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mmotm/master]

url:    https://github.com/0day-ci/linux/commits/Muchun-Song/mm-slab-Add-a-__GFP_ACCOUNT-GFP-flag-check-for-slab-allocation/20200614-144049
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/linux/kernel.h:11,
from include/linux/interrupt.h:6,
from mm/kasan/common.c:18:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
|                                          ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 |  BUG_ON(!virt_addr_valid(buf));
|  ^~~~~~
arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
|                                ^~~~~~~~~
include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
143 |  BUG_ON(!virt_addr_valid(buf));
|          ^~~~~~~~~~~~~~~
In file included from include/linux/mm_types_task.h:16,
from include/linux/mm_types.h:5,
from include/linux/mmzone.h:21,
from include/linux/topology.h:33,
from include/linux/irq.h:19,
from include/asm-generic/hardirq.h:13,
from ./arch/xtensa/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:9,
from include/linux/interrupt.h:11,
from mm/kasan/common.c:18:
mm/kasan/../internal.h: In function 'mem_map_next':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
>> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
393 |   if (!pfn_valid(pfn))
|        ^~~~~~~~~
--
In file included from arch/xtensa/include/asm/processor.h:15,
from arch/xtensa/include/asm/bitops.h:20,
from include/linux/bitops.h:19,
from mm/kasan/report.c:17:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
|                                          ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 |  BUG_ON(!virt_addr_valid(buf));
|  ^~~~~~
arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
|                                ^~~~~~~~~
include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
143 |  BUG_ON(!virt_addr_valid(buf));
|          ^~~~~~~~~~~~~~~
In file included from include/linux/mm_types_task.h:16,
from include/linux/mm_types.h:5,
from include/linux/mmzone.h:21,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from include/linux/kallsyms.h:12,
from include/linux/ftrace.h:11,
from mm/kasan/report.c:18:
mm/kasan/../internal.h: In function 'mem_map_next':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
>> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
393 |   if (!pfn_valid(pfn))
|        ^~~~~~~~~
mm/kasan/report.c: At top level:
mm/kasan/report.c:474:6: warning: no previous prototype for '__kasan_report' [-Wmissing-prototypes]
474 | void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip)
|      ^~~~~~~~~~~~~~
--
In file included from arch/xtensa/include/asm/processor.h:15,
from arch/xtensa/include/asm/bitops.h:20,
from include/linux/bitops.h:19,
from mm/kasan/generic_report.c:17:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
|                                          ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 |  BUG_ON(!virt_addr_valid(buf));
|  ^~~~~~
arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
|                                ^~~~~~~~~
include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
143 |  BUG_ON(!virt_addr_valid(buf));
|          ^~~~~~~~~~~~~~~
In file included from include/linux/mm_types_task.h:16,
from include/linux/mm_types.h:5,
from include/linux/mmzone.h:21,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from include/linux/kallsyms.h:12,
from include/linux/ftrace.h:11,
from mm/kasan/generic_report.c:18:
mm/kasan/../internal.h: In function 'mem_map_next':
arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
>> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
393 |   if (!pfn_valid(pfn))
|        ^~~~~~~~~
mm/kasan/generic_report.c: At top level:
mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load1_noabort' [-Wmissing-prototypes]
116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:129:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
129 | DEFINE_ASAN_REPORT_LOAD(1);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load2_noabort' [-Wmissing-prototypes]
116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:130:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
130 | DEFINE_ASAN_REPORT_LOAD(2);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load4_noabort' [-Wmissing-prototypes]
116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:131:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
131 | DEFINE_ASAN_REPORT_LOAD(4);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load8_noabort' [-Wmissing-prototypes]
116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:132:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
132 | DEFINE_ASAN_REPORT_LOAD(8);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load16_noabort' [-Wmissing-prototypes]
116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:133:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
133 | DEFINE_ASAN_REPORT_LOAD(16);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store1_noabort' [-Wmissing-prototypes]
123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:134:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
134 | DEFINE_ASAN_REPORT_STORE(1);
| ^~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store2_noabort' [-Wmissing-prototypes]
123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:135:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
135 | DEFINE_ASAN_REPORT_STORE(2);
| ^~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store4_noabort' [-Wmissing-prototypes]
123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:136:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
136 | DEFINE_ASAN_REPORT_STORE(4);
| ^~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store8_noabort' [-Wmissing-prototypes]
123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:137:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
137 | DEFINE_ASAN_REPORT_STORE(8);
| ^~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store16_noabort' [-Wmissing-prototypes]
123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:138:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
138 | DEFINE_ASAN_REPORT_STORE(16);
| ^~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:140:6: warning: no previous prototype for '__asan_report_load_n_noabort' [-Wmissing-prototypes]
140 | void __asan_report_load_n_noabort(unsigned long addr, size_t size)
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/kasan/generic_report.c:146:6: warning: no previous prototype for '__asan_report_store_n_noabort' [-Wmissing-prototypes]
146 | void __asan_report_store_n_noabort(unsigned long addr, size_t size)
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/pfn_valid +393 mm/kasan/../internal.h

69d177c2fc702d Andy Whitcroft  2008-11-06  383  
69d177c2fc702d Andy Whitcroft  2008-11-06  384  /*
25985edcedea63 Lucas De Marchi 2011-03-30  385   * Iterator over all subpages within the maximally aligned gigantic
69d177c2fc702d Andy Whitcroft  2008-11-06  386   * page 'base'.  Handle any discontiguity in the mem_map.
69d177c2fc702d Andy Whitcroft  2008-11-06  387   */
69d177c2fc702d Andy Whitcroft  2008-11-06  388  static inline struct page *mem_map_next(struct page *iter,
69d177c2fc702d Andy Whitcroft  2008-11-06  389  						struct page *base, int offset)
69d177c2fc702d Andy Whitcroft  2008-11-06  390  {
69d177c2fc702d Andy Whitcroft  2008-11-06  391  	if (unlikely((offset & (MAX_ORDER_NR_PAGES - 1)) == 0)) {
69d177c2fc702d Andy Whitcroft  2008-11-06  392  		unsigned long pfn = page_to_pfn(base) + offset;
69d177c2fc702d Andy Whitcroft  2008-11-06 @393  		if (!pfn_valid(pfn))
69d177c2fc702d Andy Whitcroft  2008-11-06  394  			return NULL;
69d177c2fc702d Andy Whitcroft  2008-11-06  395  		return pfn_to_page(pfn);
69d177c2fc702d Andy Whitcroft  2008-11-06  396  	}
69d177c2fc702d Andy Whitcroft  2008-11-06  397  	return iter + 1;
69d177c2fc702d Andy Whitcroft  2008-11-06  398  }
69d177c2fc702d Andy Whitcroft  2008-11-06  399  

:::::: The code at line 393 was first introduced by commit
:::::: 69d177c2fc702d402b17fdca2190d5a7e3ca55c5 hugetlbfs: handle pages higher order than MAX_ORDER

:::::: TO: Andy Whitcroft <apw@shadowen.org>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Muchun Song June 14, 2020, 11:05 a.m. UTC | #2 
On Sun, Jun 14, 2020 at 6:19 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Muchun,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on mmotm/master]
>
> url:    https://github.com/0day-ci/linux/commits/Muchun-Song/mm-slab-Add-a-__GFP_ACCOUNT-GFP-flag-check-for-slab-allocation/20200614-144049
> base:   git://git.cmpxchg.org/linux-mmotm.git master
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         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
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> In file included from include/linux/kernel.h:11,
> from include/linux/interrupt.h:6,
> from mm/kasan/common.c:18:
> include/linux/scatterlist.h: In function 'sg_set_buf':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
> 78 | # define unlikely(x) __builtin_expect(!!(x), 0)
> |                                          ^
> include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |  ^~~~~~
> arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
> 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
> |                                ^~~~~~~~~
> include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |          ^~~~~~~~~~~~~~~
> In file included from include/linux/mm_types_task.h:16,
> from include/linux/mm_types.h:5,
> from include/linux/mmzone.h:21,
> from include/linux/topology.h:33,
> from include/linux/irq.h:19,
> from include/asm-generic/hardirq.h:13,
> from ./arch/xtensa/include/generated/asm/hardirq.h:1,
> from include/linux/hardirq.h:9,
> from include/linux/interrupt.h:11,
> from mm/kasan/common.c:18:
> mm/kasan/../internal.h: In function 'mem_map_next':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
> 393 |   if (!pfn_valid(pfn))
> |        ^~~~~~~~~
> --
> In file included from arch/xtensa/include/asm/processor.h:15,
> from arch/xtensa/include/asm/bitops.h:20,
> from include/linux/bitops.h:19,
> from mm/kasan/report.c:17:
> include/linux/scatterlist.h: In function 'sg_set_buf':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
> 78 | # define unlikely(x) __builtin_expect(!!(x), 0)
> |                                          ^
> include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |  ^~~~~~
> arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
> 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
> |                                ^~~~~~~~~
> include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |          ^~~~~~~~~~~~~~~
> In file included from include/linux/mm_types_task.h:16,
> from include/linux/mm_types.h:5,
> from include/linux/mmzone.h:21,
> from include/linux/gfp.h:6,
> from include/linux/mm.h:10,
> from include/linux/kallsyms.h:12,
> from include/linux/ftrace.h:11,
> from mm/kasan/report.c:18:
> mm/kasan/../internal.h: In function 'mem_map_next':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
> 393 |   if (!pfn_valid(pfn))
> |        ^~~~~~~~~
> mm/kasan/report.c: At top level:
> mm/kasan/report.c:474:6: warning: no previous prototype for '__kasan_report' [-Wmissing-prototypes]
> 474 | void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip)
> |      ^~~~~~~~~~~~~~
> --
> In file included from arch/xtensa/include/asm/processor.h:15,
> from arch/xtensa/include/asm/bitops.h:20,
> from include/linux/bitops.h:19,
> from mm/kasan/generic_report.c:17:
> include/linux/scatterlist.h: In function 'sg_set_buf':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
> 78 | # define unlikely(x) __builtin_expect(!!(x), 0)
> |                                          ^
> include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |  ^~~~~~
> arch/xtensa/include/asm/page.h:190:32: note: in expansion of macro 'pfn_valid'
> 190 | #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
> |                                ^~~~~~~~~
> include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
> 143 |  BUG_ON(!virt_addr_valid(buf));
> |          ^~~~~~~~~~~~~~~
> In file included from include/linux/mm_types_task.h:16,
> from include/linux/mm_types.h:5,
> from include/linux/mmzone.h:21,
> from include/linux/gfp.h:6,
> from include/linux/mm.h:10,
> from include/linux/kallsyms.h:12,
> from include/linux/ftrace.h:11,
> from mm/kasan/generic_report.c:18:
> mm/kasan/../internal.h: In function 'mem_map_next':
> arch/xtensa/include/asm/page.h:182:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> 182 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
> |         ^~
> >> mm/kasan/../internal.h:393:8: note: in expansion of macro 'pfn_valid'
> 393 |   if (!pfn_valid(pfn))
> |        ^~~~~~~~~
> mm/kasan/generic_report.c: At top level:
> mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load1_noabort' [-Wmissing-prototypes]
> 116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:129:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
> 129 | DEFINE_ASAN_REPORT_LOAD(1);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load2_noabort' [-Wmissing-prototypes]
> 116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:130:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
> 130 | DEFINE_ASAN_REPORT_LOAD(2);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load4_noabort' [-Wmissing-prototypes]
> 116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:131:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
> 131 | DEFINE_ASAN_REPORT_LOAD(4);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load8_noabort' [-Wmissing-prototypes]
> 116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:132:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
> 132 | DEFINE_ASAN_REPORT_LOAD(8);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:116:6: warning: no previous prototype for '__asan_report_load16_noabort' [-Wmissing-prototypes]
> 116 | void __asan_report_load##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:133:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_LOAD'
> 133 | DEFINE_ASAN_REPORT_LOAD(16);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store1_noabort' [-Wmissing-prototypes]
> 123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:134:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
> 134 | DEFINE_ASAN_REPORT_STORE(1);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store2_noabort' [-Wmissing-prototypes]
> 123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:135:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
> 135 | DEFINE_ASAN_REPORT_STORE(2);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store4_noabort' [-Wmissing-prototypes]
> 123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:136:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
> 136 | DEFINE_ASAN_REPORT_STORE(4);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store8_noabort' [-Wmissing-prototypes]
> 123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:137:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
> 137 | DEFINE_ASAN_REPORT_STORE(8);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:123:6: warning: no previous prototype for '__asan_report_store16_noabort' [-Wmissing-prototypes]
> 123 | void __asan_report_store##size##_noabort(unsigned long addr)          |      ^~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:138:1: note: in expansion of macro 'DEFINE_ASAN_REPORT_STORE'
> 138 | DEFINE_ASAN_REPORT_STORE(16);
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:140:6: warning: no previous prototype for '__asan_report_load_n_noabort' [-Wmissing-prototypes]
> 140 | void __asan_report_load_n_noabort(unsigned long addr, size_t size)
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mm/kasan/generic_report.c:146:6: warning: no previous prototype for '__asan_report_store_n_noabort' [-Wmissing-prototypes]
> 146 | void __asan_report_store_n_noabort(unsigned long addr, size_t size)
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> vim +/pfn_valid +393 mm/kasan/../internal.h
>
> 69d177c2fc702d Andy Whitcroft  2008-11-06  383
> 69d177c2fc702d Andy Whitcroft  2008-11-06  384  /*
> 25985edcedea63 Lucas De Marchi 2011-03-30  385   * Iterator over all subpages within the maximally aligned gigantic
> 69d177c2fc702d Andy Whitcroft  2008-11-06  386   * page 'base'.  Handle any discontiguity in the mem_map.
> 69d177c2fc702d Andy Whitcroft  2008-11-06  387   */
> 69d177c2fc702d Andy Whitcroft  2008-11-06  388  static inline struct page *mem_map_next(struct page *iter,
> 69d177c2fc702d Andy Whitcroft  2008-11-06  389                                                  struct page *base, int offset)
> 69d177c2fc702d Andy Whitcroft  2008-11-06  390  {
> 69d177c2fc702d Andy Whitcroft  2008-11-06  391          if (unlikely((offset & (MAX_ORDER_NR_PAGES - 1)) == 0)) {
> 69d177c2fc702d Andy Whitcroft  2008-11-06  392                  unsigned long pfn = page_to_pfn(base) + offset;
> 69d177c2fc702d Andy Whitcroft  2008-11-06 @393                  if (!pfn_valid(pfn))
> 69d177c2fc702d Andy Whitcroft  2008-11-06  394                          return NULL;
> 69d177c2fc702d Andy Whitcroft  2008-11-06  395                  return pfn_to_page(pfn);
> 69d177c2fc702d Andy Whitcroft  2008-11-06  396          }
> 69d177c2fc702d Andy Whitcroft  2008-11-06  397          return iter + 1;
> 69d177c2fc702d Andy Whitcroft  2008-11-06  398  }
> 69d177c2fc702d Andy Whitcroft  2008-11-06  399
>
> :::::: The code at line 393 was first introduced by commit
> :::::: 69d177c2fc702d402b17fdca2190d5a7e3ca55c5 hugetlbfs: handle pages higher order than MAX_ORDER
>
> :::::: TO: Andy Whitcroft <apw@shadowen.org>
> :::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Sorry, I can't figure out where the problem is. It doesn't seem to be a problem
caused by my patch.
Michal Hocko June 15, 2020, 9:13 a.m. UTC | #3 
[Cc Roman]

On Sun 14-06-20 14:38:58, Muchun Song wrote:
> When a kmem_cache is initialized with SLAB_ACCOUNT slab flag, we must
> not call kmem_cache_alloc with __GFP_ACCOUNT GFP flag. In this case,
> we can be accounted to kmemcg twice. This is not correct. So we add a
> __GFP_ACCOUNT GFP flag check for slab allocation.
> 
> We also introduce a new helper named fixup_gfp_flags to do that check.
> We can reuse the fixup_gfp_flags for SLAB/SLUB.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/slab.c | 10 +---------
>  mm/slab.h | 21 +++++++++++++++++++++
>  mm/slub.c | 10 +---------
>  3 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 9350062ffc1a..6e0110bef2d6 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -126,8 +126,6 @@
>  
>  #include <trace/events/kmem.h>
>  
> -#include	"internal.h"
> -
>  #include	"slab.h"
>  
>  /*
> @@ -2579,13 +2577,7 @@ static struct page *cache_grow_begin(struct kmem_cache *cachep,
>  	 * Be lazy and only check for valid flags here,  keeping it out of the
>  	 * critical path in kmem_cache_alloc().
>  	 */
> -	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> -		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> -		flags &= ~GFP_SLAB_BUG_MASK;
> -		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> -				invalid_mask, &invalid_mask, flags, &flags);
> -		dump_stack();
> -	}
> +	flags = fixup_gfp_flags(cachep, flags);
>  	WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
>  	local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
>  
> diff --git a/mm/slab.h b/mm/slab.h
> index 815e4e9a94cd..0b91f2a7b033 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -109,6 +109,7 @@ struct memcg_cache_params {
>  #include <linux/kmemleak.h>
>  #include <linux/random.h>
>  #include <linux/sched/mm.h>
> +#include "internal.h"
>  
>  /*
>   * State of the slab allocator.
> @@ -627,6 +628,26 @@ struct kmem_cache_node {
>  
>  };
>  
> +static inline gfp_t fixup_gfp_flags(struct kmem_cache *s, gfp_t flags)
> +{
> +	gfp_t invalid_mask = 0;
> +
> +	if (unlikely(flags & GFP_SLAB_BUG_MASK))
> +		invalid_mask |= flags & GFP_SLAB_BUG_MASK;
> +
> +	if (unlikely(flags & __GFP_ACCOUNT && s->flags & SLAB_ACCOUNT))
> +		invalid_mask |= __GFP_ACCOUNT;
> +
> +	if (unlikely(invalid_mask)) {
> +		flags &= ~invalid_mask;
> +		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> +				invalid_mask, &invalid_mask, flags, &flags);
> +		dump_stack();
> +	}
> +
> +	return flags;
> +}
> +
>  static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
>  {
>  	return s->node[node];
> diff --git a/mm/slub.c b/mm/slub.c
> index b8f798b50d44..49b5cb7da318 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -37,8 +37,6 @@
>  
>  #include <trace/events/kmem.h>
>  
> -#include "internal.h"
> -
>  /*
>   * Lock order:
>   *   1. slab_mutex (Global Mutex)
> @@ -1745,13 +1743,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
>  
>  static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
>  {
> -	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> -		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> -		flags &= ~GFP_SLAB_BUG_MASK;
> -		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> -				invalid_mask, &invalid_mask, flags, &flags);
> -		dump_stack();
> -	}
> +	flags = fixup_gfp_flags(s, flags);
>  
>  	return allocate_slab(s,
>  		flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);
> -- 
> 2.11.0
Vlastimil Babka June 15, 2020, 1:08 p.m. UTC | #4 
On 6/14/20 8:38 AM, Muchun Song wrote:
> When a kmem_cache is initialized with SLAB_ACCOUNT slab flag, we must
> not call kmem_cache_alloc with __GFP_ACCOUNT GFP flag. In this case,
> we can be accounted to kmemcg twice. This is not correct. So we add a

Are you sure? How does that happen?

The only place I see these evaluated is this condition in slab_pre_alloc_hook():

        if (memcg_kmem_enabled() &&
            ((flags & __GFP_ACCOUNT) || (s->flags & SLAB_ACCOUNT)))
                return memcg_kmem_get_cache(s);

And it doesn't matter if one or both are set? Am I missing something?

> __GFP_ACCOUNT GFP flag check for slab allocation.
> 
> We also introduce a new helper named fixup_gfp_flags to do that check.
> We can reuse the fixup_gfp_flags for SLAB/SLUB.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/slab.c | 10 +---------
>  mm/slab.h | 21 +++++++++++++++++++++
>  mm/slub.c | 10 +---------
>  3 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 9350062ffc1a..6e0110bef2d6 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -126,8 +126,6 @@
>  
>  #include <trace/events/kmem.h>
>  
> -#include	"internal.h"
> -
>  #include	"slab.h"
>  
>  /*
> @@ -2579,13 +2577,7 @@ static struct page *cache_grow_begin(struct kmem_cache *cachep,
>  	 * Be lazy and only check for valid flags here,  keeping it out of the
>  	 * critical path in kmem_cache_alloc().
>  	 */
> -	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> -		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> -		flags &= ~GFP_SLAB_BUG_MASK;
> -		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> -				invalid_mask, &invalid_mask, flags, &flags);
> -		dump_stack();
> -	}
> +	flags = fixup_gfp_flags(cachep, flags);
>  	WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
>  	local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
>  
> diff --git a/mm/slab.h b/mm/slab.h
> index 815e4e9a94cd..0b91f2a7b033 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -109,6 +109,7 @@ struct memcg_cache_params {
>  #include <linux/kmemleak.h>
>  #include <linux/random.h>
>  #include <linux/sched/mm.h>
> +#include "internal.h"
>  
>  /*
>   * State of the slab allocator.
> @@ -627,6 +628,26 @@ struct kmem_cache_node {
>  
>  };
>  
> +static inline gfp_t fixup_gfp_flags(struct kmem_cache *s, gfp_t flags)
> +{
> +	gfp_t invalid_mask = 0;
> +
> +	if (unlikely(flags & GFP_SLAB_BUG_MASK))
> +		invalid_mask |= flags & GFP_SLAB_BUG_MASK;
> +
> +	if (unlikely(flags & __GFP_ACCOUNT && s->flags & SLAB_ACCOUNT))
> +		invalid_mask |= __GFP_ACCOUNT;
> +
> +	if (unlikely(invalid_mask)) {
> +		flags &= ~invalid_mask;
> +		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> +				invalid_mask, &invalid_mask, flags, &flags);
> +		dump_stack();
> +	}
> +
> +	return flags;
> +}
> +
>  static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
>  {
>  	return s->node[node];
> diff --git a/mm/slub.c b/mm/slub.c
> index b8f798b50d44..49b5cb7da318 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -37,8 +37,6 @@
>  
>  #include <trace/events/kmem.h>
>  
> -#include "internal.h"
> -
>  /*
>   * Lock order:
>   *   1. slab_mutex (Global Mutex)
> @@ -1745,13 +1743,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
>  
>  static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
>  {
> -	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> -		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> -		flags &= ~GFP_SLAB_BUG_MASK;
> -		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> -				invalid_mask, &invalid_mask, flags, &flags);
> -		dump_stack();
> -	}
> +	flags = fixup_gfp_flags(s, flags);
>  
>  	return allocate_slab(s,
>  		flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);
>
Muchun Song June 15, 2020, 1:32 p.m. UTC | #5 
On Mon, Jun 15, 2020 at 9:08 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 6/14/20 8:38 AM, Muchun Song wrote:
> > When a kmem_cache is initialized with SLAB_ACCOUNT slab flag, we must
> > not call kmem_cache_alloc with __GFP_ACCOUNT GFP flag. In this case,
> > we can be accounted to kmemcg twice. This is not correct. So we add a
>
> Are you sure? How does that happen?
>
> The only place I see these evaluated is this condition in slab_pre_alloc_hook():
>
>         if (memcg_kmem_enabled() &&
>             ((flags & __GFP_ACCOUNT) || (s->flags & SLAB_ACCOUNT)))
>                 return memcg_kmem_get_cache(s);
>
> And it doesn't matter if one or both are set? Am I missing something?
>
> > __GFP_ACCOUNT GFP flag check for slab allocation.
> >
> > We also introduce a new helper named fixup_gfp_flags to do that check.
> > We can reuse the fixup_gfp_flags for SLAB/SLUB.
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  mm/slab.c | 10 +---------
> >  mm/slab.h | 21 +++++++++++++++++++++
> >  mm/slub.c | 10 +---------
> >  3 files changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/mm/slab.c b/mm/slab.c
> > index 9350062ffc1a..6e0110bef2d6 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -126,8 +126,6 @@
> >
> >  #include <trace/events/kmem.h>
> >
> > -#include     "internal.h"
> > -
> >  #include     "slab.h"
> >
> >  /*
> > @@ -2579,13 +2577,7 @@ static struct page *cache_grow_begin(struct kmem_cache *cachep,
> >        * Be lazy and only check for valid flags here,  keeping it out of the
> >        * critical path in kmem_cache_alloc().
> >        */
> > -     if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> > -             gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> > -             flags &= ~GFP_SLAB_BUG_MASK;
> > -             pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> > -                             invalid_mask, &invalid_mask, flags, &flags);
> > -             dump_stack();
> > -     }
> > +     flags = fixup_gfp_flags(cachep, flags);
> >       WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
> >       local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
> >
> > diff --git a/mm/slab.h b/mm/slab.h
> > index 815e4e9a94cd..0b91f2a7b033 100644
> > --- a/mm/slab.h
> > +++ b/mm/slab.h
> > @@ -109,6 +109,7 @@ struct memcg_cache_params {
> >  #include <linux/kmemleak.h>
> >  #include <linux/random.h>
> >  #include <linux/sched/mm.h>
> > +#include "internal.h"
> >
> >  /*
> >   * State of the slab allocator.
> > @@ -627,6 +628,26 @@ struct kmem_cache_node {
> >
> >  };
> >
> > +static inline gfp_t fixup_gfp_flags(struct kmem_cache *s, gfp_t flags)
> > +{
> > +     gfp_t invalid_mask = 0;
> > +
> > +     if (unlikely(flags & GFP_SLAB_BUG_MASK))
> > +             invalid_mask |= flags & GFP_SLAB_BUG_MASK;
> > +
> > +     if (unlikely(flags & __GFP_ACCOUNT && s->flags & SLAB_ACCOUNT))
> > +             invalid_mask |= __GFP_ACCOUNT;
> > +
> > +     if (unlikely(invalid_mask)) {
> > +             flags &= ~invalid_mask;
> > +             pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> > +                             invalid_mask, &invalid_mask, flags, &flags);
> > +             dump_stack();
> > +     }
> > +
> > +     return flags;
> > +}
> > +
> >  static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
> >  {
> >       return s->node[node];
> > diff --git a/mm/slub.c b/mm/slub.c
> > index b8f798b50d44..49b5cb7da318 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -37,8 +37,6 @@
> >
> >  #include <trace/events/kmem.h>
> >
> > -#include "internal.h"
> > -
> >  /*
> >   * Lock order:
> >   *   1. slab_mutex (Global Mutex)
> > @@ -1745,13 +1743,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> >
> >  static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
> >  {
> > -     if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> > -             gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
> > -             flags &= ~GFP_SLAB_BUG_MASK;
> > -             pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> > -                             invalid_mask, &invalid_mask, flags, &flags);
> > -             dump_stack();
> > -     }
> > +     flags = fixup_gfp_flags(s, flags);
> >
> >       return allocate_slab(s,
> >               flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);
> >
>

Yeah, you are right. I'm very sorry that I was not thoughtful before.
Please ignore
this patch. Thanks!
diff mbox series

Patch

diff --git a/mm/slab.c b/mm/slab.c
index 9350062ffc1a..6e0110bef2d6 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -126,8 +126,6 @@ 
 
 #include <trace/events/kmem.h>
 
-#include	"internal.h"
-
 #include	"slab.h"
 
 /*
@@ -2579,13 +2577,7 @@  static struct page *cache_grow_begin(struct kmem_cache *cachep,
 	 * Be lazy and only check for valid flags here,  keeping it out of the
 	 * critical path in kmem_cache_alloc().
 	 */
-	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
-		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
-		flags &= ~GFP_SLAB_BUG_MASK;
-		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
-				invalid_mask, &invalid_mask, flags, &flags);
-		dump_stack();
-	}
+	flags = fixup_gfp_flags(cachep, flags);
 	WARN_ON_ONCE(cachep->ctor && (flags & __GFP_ZERO));
 	local_flags = flags & (GFP_CONSTRAINT_MASK|GFP_RECLAIM_MASK);
 
diff --git a/mm/slab.h b/mm/slab.h
index 815e4e9a94cd..0b91f2a7b033 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -109,6 +109,7 @@  struct memcg_cache_params {
 #include <linux/kmemleak.h>
 #include <linux/random.h>
 #include <linux/sched/mm.h>
+#include "internal.h"
 
 /*
  * State of the slab allocator.
@@ -627,6 +628,26 @@  struct kmem_cache_node {
 
 };
 
+static inline gfp_t fixup_gfp_flags(struct kmem_cache *s, gfp_t flags)
+{
+	gfp_t invalid_mask = 0;
+
+	if (unlikely(flags & GFP_SLAB_BUG_MASK))
+		invalid_mask |= flags & GFP_SLAB_BUG_MASK;
+
+	if (unlikely(flags & __GFP_ACCOUNT && s->flags & SLAB_ACCOUNT))
+		invalid_mask |= __GFP_ACCOUNT;
+
+	if (unlikely(invalid_mask)) {
+		flags &= ~invalid_mask;
+		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
+				invalid_mask, &invalid_mask, flags, &flags);
+		dump_stack();
+	}
+
+	return flags;
+}
+
 static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
 {
 	return s->node[node];
diff --git a/mm/slub.c b/mm/slub.c
index b8f798b50d44..49b5cb7da318 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -37,8 +37,6 @@ 
 
 #include <trace/events/kmem.h>
 
-#include "internal.h"
-
 /*
  * Lock order:
  *   1. slab_mutex (Global Mutex)
@@ -1745,13 +1743,7 @@  static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 
 static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
 {
-	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
-		gfp_t invalid_mask = flags & GFP_SLAB_BUG_MASK;
-		flags &= ~GFP_SLAB_BUG_MASK;
-		pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
-				invalid_mask, &invalid_mask, flags, &flags);
-		dump_stack();
-	}
+	flags = fixup_gfp_flags(s, flags);
 
 	return allocate_slab(s,
 		flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);