On Mon, Jun 6, 2022 at 1:20 PM Yang Shi <shy828301@xxxxxxxxx> wrote: > > On Mon, Jun 6, 2022 at 9:40 AM Zach O'Keefe <zokeefe@xxxxxxxxxx> wrote: > > > > On Sun, Jun 5, 2022 at 7:42 PM kernel test robot <lkp@xxxxxxxxx> wrote: > > > > > > Hi Zach, > > > > > > Thank you for the patch! Perhaps something to improve: > > > > > > [auto build test WARNING on akpm-mm/mm-everything] > > > > > > url: https://github.com/intel-lab-lkp/linux/commits/Zach-O-Keefe/mm-userspace-hugepage-collapse/20220606-012953 > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > > > config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220606/202206060911.I8rRqGwC-lkp@xxxxxxxxx/config) > > > compiler: gcc-11 (Debian 11.3.0-1) 11.3.0 > > > reproduce (this is a W=1 build): > > > # https://github.com/intel-lab-lkp/linux/commit/d87b6065d6050b89930cca0814921aca7c269286 > > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > > git fetch --no-tags linux-review Zach-O-Keefe/mm-userspace-hugepage-collapse/20220606-012953 > > > git checkout d87b6065d6050b89930cca0814921aca7c269286 > > > # save the config file > > > mkdir build_dir && cp config build_dir/.config > > > make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash > > > > > > If you fix the issue, kindly add following tag where applicable > > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > > > > All warnings (new ones prefixed by >>): > > > > > > mm/khugepaged.c: In function 'khugepaged': > > > >> mm/khugepaged.c:2284:1: warning: the frame size of 4160 bytes is larger than 2048 bytes [-Wframe-larger-than=] > > > 2284 | } > > > | ^ > > > > Thanks lkp@xxxxxxxxx. > > > > This is due to config with: > > > > CONFIG_FRAME_WARN=2048 > > CONFIG_NODES_SHIFT=10 > > > > Where struct collapse_control has a member int > > node_load[MAX_NUMNODES], and we stack allocate one. > > > > Is this a configuration that needs to be supported? 1024 nodes seems > > like a lot and I'm not sure if these configs are randomly generated or > > are reminiscent of real systems. > > I don't have a better idea other than moving it out of the > collapse_control struct. You may consider changing node_load to two > dimensions, for example: > > node_load[2][MAX_NUMNODES], then define: > enum { > /* khugepaged */ > COLLAPSE_ASYNC, > /* MADV_COLLAPSE */ > COLLAPSE_SYNC > } > > Then khugepaged and MADV_COLLAPSE get their dedicated node_load respectively. Sorry, I just realized this won't work for MADV_COLLAPSE since multiple processes may call it at the same time. We may consider allocating it dynamically. Have node_load the last element of collapse_control struct, then do: for_each_node(node) kmalloc(sizeof(int), GFP_KERNEL); MADV_COLLAPSE or khugepaged could just fail if it fails since THP allocation is unlikely to succeed in this case. But not sure if it is worth the complexity rather than just killing it. > > The more aggressive approach may be just killing node_load, but I'm > not sure what impact it may incur. > > > > > Thanks, > > Zach > > > > > > > > vim +2284 mm/khugepaged.c > > > > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2261 > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2262 static int khugepaged(void *none) > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2263 { > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2264 struct mm_slot *mm_slot; > > > d87b6065d6050b Zach O'Keefe 2022-06-03 2265 struct collapse_control cc = { > > > d87b6065d6050b Zach O'Keefe 2022-06-03 2266 .last_target_node = NUMA_NO_NODE, > > > d87b6065d6050b Zach O'Keefe 2022-06-03 2267 }; > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2268 > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2269 set_freezable(); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2270 set_user_nice(current, MAX_NICE); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2271 > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2272 while (!kthread_should_stop()) { > > > d87b6065d6050b Zach O'Keefe 2022-06-03 2273 khugepaged_do_scan(&cc); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2274 khugepaged_wait_work(); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2275 } > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2276 > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2277 spin_lock(&khugepaged_mm_lock); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2278 mm_slot = khugepaged_scan.mm_slot; > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2279 khugepaged_scan.mm_slot = NULL; > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2280 if (mm_slot) > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2281 collect_mm_slot(mm_slot); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2282 spin_unlock(&khugepaged_mm_lock); > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2283 return 0; > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 @2284 } > > > b46e756f5e4703 Kirill A. Shutemov 2016-07-26 2285 > > > > > > -- > > > 0-DAY CI Kernel Test Service > > > https://01.org/lkp > > >