1. Patchew
  2. linux
  3. [PATCH 0/4] mm/swap_cgroup: remove global swap cgroup lock
  4. View patch

[PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check

Kairui Song posted 4 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Kairui Song 1 year, 2 months ago 
From: Kairui Song <kasong@tencent.com>

mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
which is already checked by the caller here. Skip it by calling
__mem_cgroup_uncharge_swap() directly.

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/memcontrol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7b3503d12aaf..d3d1eb506eee 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
 		 * let's not wait for it.  The page already received a
 		 * memory+swap charge, drop the swap entry duplicate.
 		 */
-		mem_cgroup_uncharge_swap(entry, nr_pages);
+		__mem_cgroup_uncharge_swap(entry, nr_pages);
 	}
 }
 
-- 
2.47.0
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by kernel test robot 1 year, 2 months ago 
Hi Kairui,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Kairui-Song/mm-memcontrol-avoid-duplicated-memcg-enable-check/20241203-024957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20241202184154.19321-2-ryncsn%40gmail.com
patch subject: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
config: i386-buildonly-randconfig-003-20241203 (https://download.01.org/0day-ci/archive/20241203/202412031318.6qeKIH6u-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241203/202412031318.6qeKIH6u-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412031318.6qeKIH6u-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from mm/memcontrol.c:30:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from mm/memcontrol.c:56:
   include/linux/mm_inline.h:47:41: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      47 |         __mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
         |                                    ~~~~~~~~~~~ ^ ~~~
   include/linux/mm_inline.h:49:22: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
      49 |                                 NR_ZONE_LRU_BASE + lru, nr_pages);
         |                                 ~~~~~~~~~~~~~~~~ ^ ~~~
>> mm/memcontrol.c:4618:3: error: call to undeclared function '__mem_cgroup_uncharge_swap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    4618 |                 __mem_cgroup_uncharge_swap(entry, nr_pages);
         |                 ^
   mm/memcontrol.c:4618:3: note: did you mean 'mem_cgroup_uncharge_swap'?
   include/linux/swap.h:687:20: note: 'mem_cgroup_uncharge_swap' declared here
     687 | static inline void mem_cgroup_uncharge_swap(swp_entry_t entry,
         |                    ^
   3 warnings and 1 error generated.


vim +/__mem_cgroup_uncharge_swap +4618 mm/memcontrol.c

  4587	
  4588	/*
  4589	 * mem_cgroup_swapin_uncharge_swap - uncharge swap slot
  4590	 * @entry: the first swap entry for which the pages are charged
  4591	 * @nr_pages: number of pages which will be uncharged
  4592	 *
  4593	 * Call this function after successfully adding the charged page to swapcache.
  4594	 *
  4595	 * Note: This function assumes the page for which swap slot is being uncharged
  4596	 * is order 0 page.
  4597	 */
  4598	void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
  4599	{
  4600		/*
  4601		 * Cgroup1's unified memory+swap counter has been charged with the
  4602		 * new swapcache page, finish the transfer by uncharging the swap
  4603		 * slot. The swap slot would also get uncharged when it dies, but
  4604		 * it can stick around indefinitely and we'd count the page twice
  4605		 * the entire time.
  4606		 *
  4607		 * Cgroup2 has separate resource counters for memory and swap,
  4608		 * so this is a non-issue here. Memory and swap charge lifetimes
  4609		 * correspond 1:1 to page and swap slot lifetimes: we charge the
  4610		 * page to memory here, and uncharge swap when the slot is freed.
  4611		 */
  4612		if (!mem_cgroup_disabled() && do_memsw_account()) {
  4613			/*
  4614			 * The swap entry might not get freed for a long time,
  4615			 * let's not wait for it.  The page already received a
  4616			 * memory+swap charge, drop the swap entry duplicate.
  4617			 */
> 4618			__mem_cgroup_uncharge_swap(entry, nr_pages);
  4619		}
  4620	}
  4621	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by kernel test robot 1 year, 2 months ago 
Hi Kairui,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Kairui-Song/mm-memcontrol-avoid-duplicated-memcg-enable-check/20241203-024957
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20241202184154.19321-2-ryncsn%40gmail.com
patch subject: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
config: arm-randconfig-002-20241203 (https://download.01.org/0day-ci/archive/20241203/202412030915.jyKBIDck-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241203/202412030915.jyKBIDck-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412030915.jyKBIDck-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/memcontrol.c: In function 'mem_cgroup_swapin_uncharge_swap':
>> mm/memcontrol.c:4618:17: error: implicit declaration of function '__mem_cgroup_uncharge_swap'; did you mean 'mem_cgroup_uncharge_swap'? [-Wimplicit-function-declaration]
    4618 |                 __mem_cgroup_uncharge_swap(entry, nr_pages);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 mem_cgroup_uncharge_swap


vim +4618 mm/memcontrol.c

  4587	
  4588	/*
  4589	 * mem_cgroup_swapin_uncharge_swap - uncharge swap slot
  4590	 * @entry: the first swap entry for which the pages are charged
  4591	 * @nr_pages: number of pages which will be uncharged
  4592	 *
  4593	 * Call this function after successfully adding the charged page to swapcache.
  4594	 *
  4595	 * Note: This function assumes the page for which swap slot is being uncharged
  4596	 * is order 0 page.
  4597	 */
  4598	void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
  4599	{
  4600		/*
  4601		 * Cgroup1's unified memory+swap counter has been charged with the
  4602		 * new swapcache page, finish the transfer by uncharging the swap
  4603		 * slot. The swap slot would also get uncharged when it dies, but
  4604		 * it can stick around indefinitely and we'd count the page twice
  4605		 * the entire time.
  4606		 *
  4607		 * Cgroup2 has separate resource counters for memory and swap,
  4608		 * so this is a non-issue here. Memory and swap charge lifetimes
  4609		 * correspond 1:1 to page and swap slot lifetimes: we charge the
  4610		 * page to memory here, and uncharge swap when the slot is freed.
  4611		 */
  4612		if (!mem_cgroup_disabled() && do_memsw_account()) {
  4613			/*
  4614			 * The swap entry might not get freed for a long time,
  4615			 * let's not wait for it.  The page already received a
  4616			 * memory+swap charge, drop the swap entry duplicate.
  4617			 */
> 4618			__mem_cgroup_uncharge_swap(entry, nr_pages);
  4619		}
  4620	}
  4621	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Barry Song 1 year, 2 months ago 
On Tue, Dec 3, 2024 at 7:42 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> which is already checked by the caller here. Skip it by calling
> __mem_cgroup_uncharge_swap() directly.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>

Reviewed-by: Barry Song <baohua@kernel.org>

> ---
>  mm/memcontrol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 7b3503d12aaf..d3d1eb506eee 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
>                  * let's not wait for it.  The page already received a
>                  * memory+swap charge, drop the swap entry duplicate.
>                  */
> -               mem_cgroup_uncharge_swap(entry, nr_pages);
> +               __mem_cgroup_uncharge_swap(entry, nr_pages);
>         }
>  }
>
> --
> 2.47.0
>
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Roman Gushchin 1 year, 2 months ago 
On Tue, Dec 03, 2024 at 02:41:51AM +0800, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> which is already checked by the caller here. Skip it by calling
> __mem_cgroup_uncharge_swap() directly.
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>

Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>

Thanks!
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Shakeel Butt 1 year, 2 months ago 
On Tue, Dec 03, 2024 at 02:41:51AM +0800, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> which is already checked by the caller here. Skip it by calling
> __mem_cgroup_uncharge_swap() directly.
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Yosry Ahmed 1 year, 2 months ago 
On Mon, Dec 2, 2024 at 10:42 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> which is already checked by the caller here. Skip it by calling
> __mem_cgroup_uncharge_swap() directly.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/memcontrol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 7b3503d12aaf..d3d1eb506eee 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
>                  * let's not wait for it.  The page already received a
>                  * memory+swap charge, drop the swap entry duplicate.
>                  */
> -               mem_cgroup_uncharge_swap(entry, nr_pages);
> +               __mem_cgroup_uncharge_swap(entry, nr_pages);

Would it be better to instead remove the mem_cgroup_disabled() check
here and have a single check in this path?

Anyway, FWIW:

Reviewed-by: Yosry Ahmed <yosryahmed@google.com>

>         }
>  }
>
> --
> 2.47.0
>
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Kairui Song 1 year, 2 months ago 
On Tue, Dec 3, 2024 at 3:11 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Mon, Dec 2, 2024 at 10:42 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > From: Kairui Song <kasong@tencent.com>
> >
> > mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> > which is already checked by the caller here. Skip it by calling
> > __mem_cgroup_uncharge_swap() directly.
> >
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > ---
> >  mm/memcontrol.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 7b3503d12aaf..d3d1eb506eee 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> >                  * let's not wait for it.  The page already received a
> >                  * memory+swap charge, drop the swap entry duplicate.
> >                  */
> > -               mem_cgroup_uncharge_swap(entry, nr_pages);
> > +               __mem_cgroup_uncharge_swap(entry, nr_pages);
>
> Would it be better to instead remove the mem_cgroup_disabled() check
> here and have a single check in this path?

Good suggestion, and the kernel test bot just reported
__mem_cgroup_uncharge_swap is undefined with !CONFIG_SWAP, so better
to fix it by removing the check instead.

>
> Anyway, FWIW:
>
> Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
>
> >         }
> >  }
> >
> > --
> > 2.47.0
> >
>
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Shakeel Butt 1 year, 2 months ago 
On Tue, Dec 03, 2024 at 04:25:57PM +0800, Kairui Song wrote:
> On Tue, Dec 3, 2024 at 3:11 AM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > On Mon, Dec 2, 2024 at 10:42 AM Kairui Song <ryncsn@gmail.com> wrote:
> > >
> > > From: Kairui Song <kasong@tencent.com>
> > >
> > > mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> > > which is already checked by the caller here. Skip it by calling
> > > __mem_cgroup_uncharge_swap() directly.
> > >
> > > Signed-off-by: Kairui Song <kasong@tencent.com>
> > > ---
> > >  mm/memcontrol.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index 7b3503d12aaf..d3d1eb506eee 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> > >                  * let's not wait for it.  The page already received a
> > >                  * memory+swap charge, drop the swap entry duplicate.
> > >                  */
> > > -               mem_cgroup_uncharge_swap(entry, nr_pages);
> > > +               __mem_cgroup_uncharge_swap(entry, nr_pages);
> >
> > Would it be better to instead remove the mem_cgroup_disabled() check
> > here and have a single check in this path?
> 
> Good suggestion, and the kernel test bot just reported
> __mem_cgroup_uncharge_swap is undefined with !CONFIG_SWAP, so better
> to fix it by removing the check instead.
> 

This sounds reasonable.
Re: [PATCH 1/4] mm, memcontrol: avoid duplicated memcg enable check
Posted by Chris Li 1 year, 2 months ago 
Hi Kairui,

On Tue, Dec 3, 2024 at 12:26 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Tue, Dec 3, 2024 at 3:11 AM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > On Mon, Dec 2, 2024 at 10:42 AM Kairui Song <ryncsn@gmail.com> wrote:
> > >
> > > From: Kairui Song <kasong@tencent.com>
> > >
> > > mem_cgroup_uncharge_swap() implies a mem_cgroup_disabled() check,
> > > which is already checked by the caller here. Skip it by calling
> > > __mem_cgroup_uncharge_swap() directly.
> > >
> > > Signed-off-by: Kairui Song <kasong@tencent.com>
> > > ---
> > >  mm/memcontrol.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index 7b3503d12aaf..d3d1eb506eee 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -4615,7 +4615,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> > >                  * let's not wait for it.  The page already received a
> > >                  * memory+swap charge, drop the swap entry duplicate.
> > >                  */
> > > -               mem_cgroup_uncharge_swap(entry, nr_pages);
> > > +               __mem_cgroup_uncharge_swap(entry, nr_pages);
> >
> > Would it be better to instead remove the mem_cgroup_disabled() check
> > here and have a single check in this path?
>
> Good suggestion, and the kernel test bot just reported
> __mem_cgroup_uncharge_swap is undefined with !CONFIG_SWAP, so better
> to fix it by removing the check instead.

Agree with Yosry on the suggestion of calling
mem_cgroup_uncharge_swap() instead. With that.

Acked-by: Chris Li <chrisl@kernel.org>

Chris

>
> >
> > Anyway, FWIW:
> >
> > Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
> >
> > >         }
> > >  }
> > >
> > > --
> > > 2.47.0
> > >
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.