Message ID | 20230228085002.2592473-3-yosryahmed@google.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Ignore non-LRU-based reclaim in memcg reclaim | expand |
Hi Yosry, Thank you for the patch! Yet something to improve: [auto build test ERROR on akpm-mm/mm-everything] [cannot apply to vbabka-slab/for-next xfs-linux/for-next v6.2] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230228085002.2592473-3-yosryahmed%40google.com patch subject: [PATCH v1 2/2] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim config: x86_64-randconfig-a014-20230227 (https://download.01.org/0day-ci/archive/20230228/202302281959.EmOJaeae-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/f6d2b849f186a927925a29e289d60895048550f5 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 git checkout f6d2b849f186a927925a29e289d60895048550f5 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 olddefconfig 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@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202302281959.EmOJaeae-lkp@intel.com/ All errors (new ones prefixed by >>): >> mm/vmscan.c:549:13: error: redefinition of 'cgroup_reclaim' 549 | static bool cgroup_reclaim(struct scan_control *sc) | ^~~~~~~~~~~~~~ mm/vmscan.c:191:13: note: previous definition of 'cgroup_reclaim' with type 'bool(struct scan_control *)' {aka '_Bool(struct scan_control *)'} 191 | static bool cgroup_reclaim(struct scan_control *sc) | ^~~~~~~~~~~~~~ >> mm/vmscan.c:554:13: error: redefinition of 'global_reclaim' 554 | static bool global_reclaim(struct scan_control *sc) | ^~~~~~~~~~~~~~ mm/vmscan.c:196:13: note: previous definition of 'global_reclaim' with type 'bool(struct scan_control *)' {aka '_Bool(struct scan_control *)'} 196 | static bool global_reclaim(struct scan_control *sc) | ^~~~~~~~~~~~~~ mm/vmscan.c:196:13: warning: 'global_reclaim' defined but not used [-Wunused-function] vim +/cgroup_reclaim +549 mm/vmscan.c 86750830468506 Yang Shi 2021-05-04 548 b5ead35e7e1d34 Johannes Weiner 2019-11-30 @549 static bool cgroup_reclaim(struct scan_control *sc) 89b5fae5368f6a Johannes Weiner 2012-01-12 550 { b5ead35e7e1d34 Johannes Weiner 2019-11-30 551 return false; 89b5fae5368f6a Johannes Weiner 2012-01-12 552 } 97c9341f727105 Tejun Heo 2015-05-22 553 a579086c99ed70 Yu Zhao 2022-12-21 @554 static bool global_reclaim(struct scan_control *sc) a579086c99ed70 Yu Zhao 2022-12-21 555 { a579086c99ed70 Yu Zhao 2022-12-21 556 return true; a579086c99ed70 Yu Zhao 2022-12-21 557 } a579086c99ed70 Yu Zhao 2022-12-21 558
Hi Yosry, Thank you for the patch! Yet something to improve: [auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on linus/master next-20230228] [cannot apply to vbabka-slab/for-next xfs-linux/for-next v6.2] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230228085002.2592473-3-yosryahmed%40google.com patch subject: [PATCH v1 2/2] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim config: i386-randconfig-a002-20230227 (https://download.01.org/0day-ci/archive/20230228/202302281933.vU1PHuZr-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) 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 # https://github.com/intel-lab-lkp/linux/commit/f6d2b849f186a927925a29e289d60895048550f5 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 git checkout f6d2b849f186a927925a29e289d60895048550f5 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202302281933.vU1PHuZr-lkp@intel.com/ All errors (new ones prefixed by >>): >> mm/vmscan.c:549:13: error: redefinition of 'cgroup_reclaim' static bool cgroup_reclaim(struct scan_control *sc) ^ mm/vmscan.c:191:13: note: previous definition is here static bool cgroup_reclaim(struct scan_control *sc) ^ >> mm/vmscan.c:554:13: error: redefinition of 'global_reclaim' static bool global_reclaim(struct scan_control *sc) ^ mm/vmscan.c:196:13: note: previous definition is here static bool global_reclaim(struct scan_control *sc) ^ 2 errors generated. vim +/cgroup_reclaim +549 mm/vmscan.c 86750830468506 Yang Shi 2021-05-04 548 b5ead35e7e1d34 Johannes Weiner 2019-11-30 @549 static bool cgroup_reclaim(struct scan_control *sc) 89b5fae5368f6a Johannes Weiner 2012-01-12 550 { b5ead35e7e1d34 Johannes Weiner 2019-11-30 551 return false; 89b5fae5368f6a Johannes Weiner 2012-01-12 552 } 97c9341f727105 Tejun Heo 2015-05-22 553 a579086c99ed70 Yu Zhao 2022-12-21 @554 static bool global_reclaim(struct scan_control *sc) a579086c99ed70 Yu Zhao 2022-12-21 555 { a579086c99ed70 Yu Zhao 2022-12-21 556 return true; a579086c99ed70 Yu Zhao 2022-12-21 557 } a579086c99ed70 Yu Zhao 2022-12-21 558
On Tue, Feb 28, 2023 at 3:56 AM kernel test robot <lkp@intel.com> wrote: > > Hi Yosry, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on akpm-mm/mm-everything] > [also build test ERROR on linus/master next-20230228] > [cannot apply to vbabka-slab/for-next xfs-linux/for-next v6.2] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > patch link: https://lore.kernel.org/r/20230228085002.2592473-3-yosryahmed%40google.com > patch subject: [PATCH v1 2/2] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim > config: i386-randconfig-a002-20230227 (https://download.01.org/0day-ci/archive/20230228/202302281933.vU1PHuZr-lkp@intel.com/config) > compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) > 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 > # https://github.com/intel-lab-lkp/linux/commit/f6d2b849f186a927925a29e289d60895048550f5 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 > git checkout f6d2b849f186a927925a29e289d60895048550f5 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Link: https://lore.kernel.org/oe-kbuild-all/202302281933.vU1PHuZr-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > >> mm/vmscan.c:549:13: error: redefinition of 'cgroup_reclaim' > static bool cgroup_reclaim(struct scan_control *sc) > ^ > mm/vmscan.c:191:13: note: previous definition is here > static bool cgroup_reclaim(struct scan_control *sc) > ^ > >> mm/vmscan.c:554:13: error: redefinition of 'global_reclaim' > static bool global_reclaim(struct scan_control *sc) > ^ > mm/vmscan.c:196:13: note: previous definition is here > static bool global_reclaim(struct scan_control *sc) > ^ > 2 errors generated. Ugh yeah I didn't realize I am moving the definitions from within an #ifdef CONFIG_MEMCG. I will just leave the definitions as-is and add a forward declaration before the definition of add_non_vmscan_reclaimed(), should also reduce the churn in the diff. Will wait for a bit before re-spinning to gather some feedback on the current version first. > > > vim +/cgroup_reclaim +549 mm/vmscan.c > > 86750830468506 Yang Shi 2021-05-04 548 > b5ead35e7e1d34 Johannes Weiner 2019-11-30 @549 static bool cgroup_reclaim(struct scan_control *sc) > 89b5fae5368f6a Johannes Weiner 2012-01-12 550 { > b5ead35e7e1d34 Johannes Weiner 2019-11-30 551 return false; > 89b5fae5368f6a Johannes Weiner 2012-01-12 552 } > 97c9341f727105 Tejun Heo 2015-05-22 553 > a579086c99ed70 Yu Zhao 2022-12-21 @554 static bool global_reclaim(struct scan_control *sc) > a579086c99ed70 Yu Zhao 2022-12-21 555 { > a579086c99ed70 Yu Zhao 2022-12-21 556 return true; > a579086c99ed70 Yu Zhao 2022-12-21 557 } > a579086c99ed70 Yu Zhao 2022-12-21 558 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests
On Tue, Feb 28, 2023 at 9:18 AM Yosry Ahmed <yosryahmed@google.com> wrote: > > On Tue, Feb 28, 2023 at 3:56 AM kernel test robot <lkp@intel.com> wrote: > > > > Hi Yosry, > > > > Thank you for the patch! Yet something to improve: > > > > [auto build test ERROR on akpm-mm/mm-everything] > > [also build test ERROR on linus/master next-20230228] > > [cannot apply to vbabka-slab/for-next xfs-linux/for-next v6.2] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > > patch link: https://lore.kernel.org/r/20230228085002.2592473-3-yosryahmed%40google.com > > patch subject: [PATCH v1 2/2] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim > > config: i386-randconfig-a002-20230227 (https://download.01.org/0day-ci/archive/20230228/202302281933.vU1PHuZr-lkp@intel.com/config) > > compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) > > 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 > > # https://github.com/intel-lab-lkp/linux/commit/f6d2b849f186a927925a29e289d60895048550f5 > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > git fetch --no-tags linux-review Yosry-Ahmed/mm-vmscan-refactor-updating-reclaimed-pages-in-reclaim_state/20230228-165214 > > git checkout f6d2b849f186a927925a29e289d60895048550f5 > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash > > > > If you fix the issue, kindly add following tag where applicable > > | Reported-by: kernel test robot <lkp@intel.com> > > | Link: https://lore.kernel.org/oe-kbuild-all/202302281933.vU1PHuZr-lkp@intel.com/ > > > > All errors (new ones prefixed by >>): > > > > >> mm/vmscan.c:549:13: error: redefinition of 'cgroup_reclaim' > > static bool cgroup_reclaim(struct scan_control *sc) > > ^ > > mm/vmscan.c:191:13: note: previous definition is here > > static bool cgroup_reclaim(struct scan_control *sc) > > ^ > > >> mm/vmscan.c:554:13: error: redefinition of 'global_reclaim' > > static bool global_reclaim(struct scan_control *sc) > > ^ > > mm/vmscan.c:196:13: note: previous definition is here > > static bool global_reclaim(struct scan_control *sc) > > ^ > > 2 errors generated. > > Ugh yeah I didn't realize I am moving the definitions from within an > #ifdef CONFIG_MEMCG. I will just leave the definitions as-is and add a > forward declaration before the definition of > add_non_vmscan_reclaimed(), should also reduce the churn in the diff. > Will wait for a bit before re-spinning to gather some feedback on the > current version first. I can also just move all the reclaim state functions (set_task_reclaim_state(), report_freed_pages(), add_non_vmscan_reclaimed()) below that #ifdef CONFIG_MEMCG. Might also name them more consistently. > > > > > > > vim +/cgroup_reclaim +549 mm/vmscan.c > > > > 86750830468506 Yang Shi 2021-05-04 548 > > b5ead35e7e1d34 Johannes Weiner 2019-11-30 @549 static bool cgroup_reclaim(struct scan_control *sc) > > 89b5fae5368f6a Johannes Weiner 2012-01-12 550 { > > b5ead35e7e1d34 Johannes Weiner 2019-11-30 551 return false; > > 89b5fae5368f6a Johannes Weiner 2012-01-12 552 } > > 97c9341f727105 Tejun Heo 2015-05-22 553 > > a579086c99ed70 Yu Zhao 2022-12-21 @554 static bool global_reclaim(struct scan_control *sc) > > a579086c99ed70 Yu Zhao 2022-12-21 555 { > > a579086c99ed70 Yu Zhao 2022-12-21 556 return true; > > a579086c99ed70 Yu Zhao 2022-12-21 557 } > > a579086c99ed70 Yu Zhao 2022-12-21 558 > > > > -- > > 0-DAY CI Kernel Test Service > > https://github.com/intel/lkp-tests
diff --git a/mm/vmscan.c b/mm/vmscan.c index 8846531e85a4..c53659221965 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -188,6 +188,16 @@ struct scan_control { */ int vm_swappiness = 60; +static bool cgroup_reclaim(struct scan_control *sc) +{ + return sc->target_mem_cgroup; +} + +static bool global_reclaim(struct scan_control *sc) +{ + return !sc->target_mem_cgroup || mem_cgroup_is_root(sc->target_mem_cgroup); +} + static void set_task_reclaim_state(struct task_struct *task, struct reclaim_state *rs) { @@ -217,7 +227,35 @@ EXPORT_SYMBOL(report_freed_pages); static void add_non_vmscan_reclaimed(struct scan_control *sc, struct reclaim_state *rs) { - if (rs) { + /* + * Currently, reclaim_state->reclaimed includes three types of pages + * freed outside of vmscan: + * (1) Slab pages. + * (2) Clean file pages from pruned inodes. + * (3) XFS freed buffer pages. + * + * For all of these cases, we have no way of finding out whether these + * pages were related to the memcg under reclaim. For example, a freed + * slab page could have had only a single object charged to the memcg + * under reclaim. Also, populated inodes are not on shrinker LRUs + * anymore except on highmem systems. + * + * Instead of over-reporting the reclaimed pages in a memcg reclaim, + * only count such pages in system-wide reclaim. This prevents + * unnecessary retries during memcg charging and false positive from + * proactive reclaim (memory.reclaim). + * + * For uncommon cases were the freed pages were actually significantly + * charged to the memcg under reclaim, and we end up under-reporting, it + * should be fine. The freed pages will be uncharged anyway, even if + * they are not reported properly, and we will be able to make forward + * progress in charging (which is usually in a retry loop). + * + * We can go one step further, and report the uncharged objcg pages in + * memcg reclaim, to make reporting more accurate and reduce + * under-reporting, but it's probably not worth the complexity for now. + */ + if (rs && !cgroup_reclaim(sc)) { sc->nr_reclaimed += rs->reclaimed; rs->reclaimed = 0; } @@ -463,16 +501,6 @@ void reparent_shrinker_deferred(struct mem_cgroup *memcg) up_read(&shrinker_rwsem); } -static bool cgroup_reclaim(struct scan_control *sc) -{ - return sc->target_mem_cgroup; -} - -static bool global_reclaim(struct scan_control *sc) -{ - return !sc->target_mem_cgroup || mem_cgroup_is_root(sc->target_mem_cgroup); -} - /** * writeback_throttling_sane - is the usual dirty throttling mechanism available? * @sc: scan_control in question
We keep track of different types of reclaimed pages through reclaim_state->reclaimed, and we add them to the reported number of reclaimed pages. For non-memcg reclaim, this makes sense. For memcg reclaim, we have no clue if those pages are charged to the memcg under reclaim. Slab pages are shared by different memcgs, so a freed slab page may have only been partially charged to the memcg under reclaim. The same goes for clean file pages from pruned inodes (on highmem systems) or xfs buffer pages, there is no way to link them to the memcg under reclaim. Stop reporting those freed pages as reclaimed pages during memcg reclaim. This should make the return value of writing to memory.reclaim, and may help reduce unnecessary reclaim retries during memcg charging. Generally, this should make the return value of try_to_free_mem_cgroup_pages() more accurate. In some limited cases (e.g. freed a slab page that was mostly charged to the memcg under reclaim), the return value of try_to_free_mem_cgroup_pages() can be underestimated, but this should be fine. The freed pages will be uncharged anyway, and we can charge the memcg the next time around as we usually do memcg reclaim in a retry loop. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/vmscan.c | 50 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 11 deletions(-)