[PATCH 0/3] mm/memcg: Miscellaneous cleanups and streamlining

[PATCH 0/3] mm/memcg: Miscellaneous cleanups and streamlining

[Waiman Long]

Patch 1 removes an unused enum charge_type.

Patch 2 streamlines mem_cgroup_get_max().

Patch 3 unifies swap and memsw page counters in mem_cgroup.

Waiman Long (3):
  mm/memcg: Clean up obsolete enum charge_type
  mm/memcg: Simplify mem_cgroup_get_max()
  mm/memcg: Unify swap and memsw page counters

 include/linux/memcontrol.h |  3 +--
 mm/memcontrol.c            | 30 ++++++++++--------------------
 2 files changed, 11 insertions(+), 22 deletions(-)

-- 
2.18.1

[PATCH 1/3] mm/memcg: Clean up obsolete enum charge_type

[Waiman Long]

Since commit 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API") and
commit 00501b531c47 ("mm: memcontrol: rewrite charge API") in v3.17, the
enum charge_type was no longer used anywhere. However, the enum itself
was not removed at that time. Remove the obsolete enum charge_type now.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 mm/memcontrol.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b807952b4d43..26b7a48d3afb 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -197,14 +197,6 @@ static struct move_charge_struct {
 #define	MEM_CGROUP_MAX_RECLAIM_LOOPS		100
 #define	MEM_CGROUP_MAX_SOFT_LIMIT_RECLAIM_LOOPS	2
 
-enum charge_type {
-	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,
-	MEM_CGROUP_CHARGE_TYPE_ANON,
-	MEM_CGROUP_CHARGE_TYPE_SWAPOUT,	/* for accounting swapcache */
-	MEM_CGROUP_CHARGE_TYPE_DROP,	/* a page was unused swap cache */
-	NR_CHARGE_TYPE,
-};
-
 /* for encoding cft->private value on file */
 enum res_type {
 	_MEM,
-- 
2.18.1

Re: [PATCH 1/3] mm/memcg: Clean up obsolete enum charge_type

[Johannes Weiner]

On Thu, Aug 20, 2020 at 09:03:48AM -0400, Waiman Long wrote:
> Since commit 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API") and
> commit 00501b531c47 ("mm: memcontrol: rewrite charge API") in v3.17, the
> enum charge_type was no longer used anywhere. However, the enum itself
> was not removed at that time. Remove the obsolete enum charge_type now.
> 
> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>

Re: [PATCH 1/3] mm/memcg: Clean up obsolete enum charge_type

[Shakeel Butt]

On Thu, Aug 20, 2020 at 6:04 AM Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> Since commit 0a31bc97c80c ("mm: memcontrol: rewrite uncharge API") and
> commit 00501b531c47 ("mm: memcontrol: rewrite charge API") in v3.17, the
> enum charge_type was no longer used anywhere. However, the enum itself
> was not removed at that time. Remove the obsolete enum charge_type now.
>
> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

[PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max()

[Waiman Long]

The mem_cgroup_get_max() function used to get memory+swap max from
both the v1 memsw and v2 memory+swap page counters & return the maximum
of these 2 values. This is redundant and it is more efficient to just
get either the v1 or the v2 values depending on which one is currently
in use.

Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 mm/memcontrol.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 26b7a48d3afb..d219dca5239f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
  */
 unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg)
 {
-	unsigned long max;
+	unsigned long max = READ_ONCE(memcg->memory.max);
 
-	max = READ_ONCE(memcg->memory.max);
 	if (mem_cgroup_swappiness(memcg)) {
-		unsigned long memsw_max;
-		unsigned long swap_max;
-
-		memsw_max = memcg->memsw.max;
-		swap_max = READ_ONCE(memcg->swap.max);
-		swap_max = min(swap_max, (unsigned long)total_swap_pages);
-		max = min(max + swap_max, memsw_max);
+		if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
+			max += READ_ONCE(memcg->swap.max);
+		else
+			max = memcg->memsw.max;
 	}
 	return max;
 }
-- 
2.18.1

Re: [PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max()

[Johannes Weiner]

On Thu, Aug 20, 2020 at 09:03:49AM -0400, Waiman Long wrote:
> The mem_cgroup_get_max() function used to get memory+swap max from
> both the v1 memsw and v2 memory+swap page counters & return the maximum
> of these 2 values. This is redundant and it is more efficient to just
> get either the v1 or the v2 values depending on which one is currently
> in use.
> 
> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  mm/memcontrol.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 26b7a48d3afb..d219dca5239f 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
>   */
>  unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg)
>  {
> -	unsigned long max;
> +	unsigned long max = READ_ONCE(memcg->memory.max);
>  
> -	max = READ_ONCE(memcg->memory.max);
>  	if (mem_cgroup_swappiness(memcg)) {
> -		unsigned long memsw_max;
> -		unsigned long swap_max;
> -
> -		memsw_max = memcg->memsw.max;
> -		swap_max = READ_ONCE(memcg->swap.max);
> -		swap_max = min(swap_max, (unsigned long)total_swap_pages);
> -		max = min(max + swap_max, memsw_max);
> +		if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
> +			max += READ_ONCE(memcg->swap.max);
> +		else
> +			max = memcg->memsw.max;

I agree with the premise of the patch, but v1 and v2 have sufficiently
different logic, and the way v1 overrides max from the innermost
branch again also doesn't help in understanding what's going on.

Can you please split out the v1 and v2 code?

	if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
		max = READ_ONCE(memcg->memory.max);
		if (mem_cgroup_swappiness(memcg))
			max += READ_ONCE(memcg->swap.max);
	} else {
		if (mem_cgroup_swappiness(memcg))
			max = memcg->memsw.max;
		else
			max = READ_ONCE(memcg->memory.max);
	}

It's slightly repetitive, but IMO much more readable.

Re: [PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max()

[Waiman Long]

On 8/20/20 1:35 PM, Johannes Weiner wrote:
> On Thu, Aug 20, 2020 at 09:03:49AM -0400, Waiman Long wrote:
>> The mem_cgroup_get_max() function used to get memory+swap max from
>> both the v1 memsw and v2 memory+swap page counters & return the maximum
>> of these 2 values. This is redundant and it is more efficient to just
>> get either the v1 or the v2 values depending on which one is currently
>> in use.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   mm/memcontrol.c | 14 +++++---------
>>   1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 26b7a48d3afb..d219dca5239f 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
>>    */
>>   unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg)
>>   {
>> -	unsigned long max;
>> +	unsigned long max = READ_ONCE(memcg->memory.max);
>>   
>> -	max = READ_ONCE(memcg->memory.max);
>>   	if (mem_cgroup_swappiness(memcg)) {
>> -		unsigned long memsw_max;
>> -		unsigned long swap_max;
>> -
>> -		memsw_max = memcg->memsw.max;
>> -		swap_max = READ_ONCE(memcg->swap.max);
>> -		swap_max = min(swap_max, (unsigned long)total_swap_pages);
>> -		max = min(max + swap_max, memsw_max);
>> +		if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
>> +			max += READ_ONCE(memcg->swap.max);
>> +		else
>> +			max = memcg->memsw.max;
> I agree with the premise of the patch, but v1 and v2 have sufficiently
> different logic, and the way v1 overrides max from the innermost
> branch again also doesn't help in understanding what's going on.
>
> Can you please split out the v1 and v2 code?
>
> 	if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
> 		max = READ_ONCE(memcg->memory.max);
> 		if (mem_cgroup_swappiness(memcg))
> 			max += READ_ONCE(memcg->swap.max);
> 	} else {
> 		if (mem_cgroup_swappiness(memcg))
> 			max = memcg->memsw.max;
> 		else
> 			max = READ_ONCE(memcg->memory.max);
> 	}
>
> It's slightly repetitive, but IMO much more readable.
>
Sure. That makes it even better.

Cheers,
Longman

Re: [PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max()

[Shakeel Butt]

On Thu, Aug 20, 2020 at 1:29 PM Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> On 8/20/20 1:35 PM, Johannes Weiner wrote:
> > On Thu, Aug 20, 2020 at 09:03:49AM -0400, Waiman Long wrote:
> >> The mem_cgroup_get_max() function used to get memory+swap max from
> >> both the v1 memsw and v2 memory+swap page counters & return the maximum
> >> of these 2 values. This is redundant and it is more efficient to just
> >> get either the v1 or the v2 values depending on which one is currently
> >> in use.
> >>
> >> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> ---
> >>   mm/memcontrol.c | 14 +++++---------
> >>   1 file changed, 5 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 26b7a48d3afb..d219dca5239f 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
> >>    */
> >>   unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg)
> >>   {
> >> -    unsigned long max;
> >> +    unsigned long max = READ_ONCE(memcg->memory.max);
> >>
> >> -    max = READ_ONCE(memcg->memory.max);
> >>      if (mem_cgroup_swappiness(memcg)) {
> >> -            unsigned long memsw_max;
> >> -            unsigned long swap_max;
> >> -
> >> -            memsw_max = memcg->memsw.max;
> >> -            swap_max = READ_ONCE(memcg->swap.max);
> >> -            swap_max = min(swap_max, (unsigned long)total_swap_pages);
> >> -            max = min(max + swap_max, memsw_max);
> >> +            if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
> >> +                    max += READ_ONCE(memcg->swap.max);
> >> +            else
> >> +                    max = memcg->memsw.max;
> > I agree with the premise of the patch, but v1 and v2 have sufficiently
> > different logic, and the way v1 overrides max from the innermost
> > branch again also doesn't help in understanding what's going on.
> >
> > Can you please split out the v1 and v2 code?
> >
> >       if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
> >               max = READ_ONCE(memcg->memory.max);
> >               if (mem_cgroup_swappiness(memcg))
> >                       max += READ_ONCE(memcg->swap.max);
> >       } else {
> >               if (mem_cgroup_swappiness(memcg))
> >                       max = memcg->memsw.max;
> >               else
> >                       max = READ_ONCE(memcg->memory.max);
> >       }
> >
> > It's slightly repetitive, but IMO much more readable.
> >
> Sure. That makes it even better.
>

Can you please also add in the commit message why it is ok to drop
total_swap_pages comparison from mem_cgroup_get_max()?

Re: [PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max()

[Waiman Long]

On 8/20/20 5:25 PM, Shakeel Butt wrote:
> On Thu, Aug 20, 2020 at 1:29 PM Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On 8/20/20 1:35 PM, Johannes Weiner wrote:
>>> On Thu, Aug 20, 2020 at 09:03:49AM -0400, Waiman Long wrote:
>>>> The mem_cgroup_get_max() function used to get memory+swap max from
>>>> both the v1 memsw and v2 memory+swap page counters & return the maximum
>>>> of these 2 values. This is redundant and it is more efficient to just
>>>> get either the v1 or the v2 values depending on which one is currently
>>>> in use.
>>>>
>>>> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    mm/memcontrol.c | 14 +++++---------
>>>>    1 file changed, 5 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index 26b7a48d3afb..d219dca5239f 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>>> @@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
>>>>     */
>>>>    unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg)
>>>>    {
>>>> -    unsigned long max;
>>>> +    unsigned long max = READ_ONCE(memcg->memory.max);
>>>>
>>>> -    max = READ_ONCE(memcg->memory.max);
>>>>       if (mem_cgroup_swappiness(memcg)) {
>>>> -            unsigned long memsw_max;
>>>> -            unsigned long swap_max;
>>>> -
>>>> -            memsw_max = memcg->memsw.max;
>>>> -            swap_max = READ_ONCE(memcg->swap.max);
>>>> -            swap_max = min(swap_max, (unsigned long)total_swap_pages);
>>>> -            max = min(max + swap_max, memsw_max);
>>>> +            if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
>>>> +                    max += READ_ONCE(memcg->swap.max);
>>>> +            else
>>>> +                    max = memcg->memsw.max;
>>> I agree with the premise of the patch, but v1 and v2 have sufficiently
>>> different logic, and the way v1 overrides max from the innermost
>>> branch again also doesn't help in understanding what's going on.
>>>
>>> Can you please split out the v1 and v2 code?
>>>
>>>        if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
>>>                max = READ_ONCE(memcg->memory.max);
>>>                if (mem_cgroup_swappiness(memcg))
>>>                        max += READ_ONCE(memcg->swap.max);
>>>        } else {
>>>                if (mem_cgroup_swappiness(memcg))
>>>                        max = memcg->memsw.max;
>>>                else
>>>                        max = READ_ONCE(memcg->memory.max);
>>>        }
>>>
>>> It's slightly repetitive, but IMO much more readable.
>>>
>> Sure. That makes it even better.
>>
> Can you please also add in the commit message why it is ok to drop
> total_swap_pages comparison from mem_cgroup_get_max()?
>
My bad. I accidentally skipped the total_swap_pages check. Will add it 
back in v2.

Cheers,
Longman

[PATCH 3/3] mm/memcg: Unify swap and memsw page counters

[Waiman Long]

The swap page counter is v2 only while memsw is v1 only. As v1 and v2
controllers cannot be active at the same time, there is no point to keep
both swap and memsw page counters in mem_cgroup. The previous patch has
made sure that memsw page counter is updated and accessed only when in
v1 code paths. So it is now safe to alias the v1 memsw page counter to v2
swap page counter. This saves 14 long's in the size of mem_cgroup. This
is a saving of 112 bytes for 64-bit archs.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/memcontrol.h | 3 +--
 mm/memcontrol.c            | 8 +++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d0b036123c6a..d2a819d7db70 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -216,10 +216,9 @@ struct mem_cgroup {
 
 	/* Accounted resources */
 	struct page_counter memory;
-	struct page_counter swap;
+	struct page_counter swap;	/* memsw (memory+swap) for v1 */
 
 	/* Legacy consumer-oriented counters */
-	struct page_counter memsw;
 	struct page_counter kmem;
 	struct page_counter tcpmem;
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index d219dca5239f..04c3794cdc98 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -68,6 +68,11 @@
 
 #include <trace/events/vmscan.h>
 
+/*
+ * The v1 memsw page counter is aliased to the v2 swap page counter.
+ */
+#define memsw	swap
+
 struct cgroup_subsys memory_cgrp_subsys __read_mostly;
 EXPORT_SYMBOL(memory_cgrp_subsys);
 
@@ -5279,13 +5284,11 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 		memcg->use_hierarchy = true;
 		page_counter_init(&memcg->memory, &parent->memory);
 		page_counter_init(&memcg->swap, &parent->swap);
-		page_counter_init(&memcg->memsw, &parent->memsw);
 		page_counter_init(&memcg->kmem, &parent->kmem);
 		page_counter_init(&memcg->tcpmem, &parent->tcpmem);
 	} else {
 		page_counter_init(&memcg->memory, NULL);
 		page_counter_init(&memcg->swap, NULL);
-		page_counter_init(&memcg->memsw, NULL);
 		page_counter_init(&memcg->kmem, NULL);
 		page_counter_init(&memcg->tcpmem, NULL);
 		/*
@@ -5414,7 +5417,6 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css)
 
 	page_counter_set_max(&memcg->memory, PAGE_COUNTER_MAX);
 	page_counter_set_max(&memcg->swap, PAGE_COUNTER_MAX);
-	page_counter_set_max(&memcg->memsw, PAGE_COUNTER_MAX);
 	page_counter_set_max(&memcg->kmem, PAGE_COUNTER_MAX);
 	page_counter_set_max(&memcg->tcpmem, PAGE_COUNTER_MAX);
 	page_counter_set_min(&memcg->memory, 0);
-- 
2.18.1

Re: [PATCH 3/3] mm/memcg: Unify swap and memsw page counters

[Shakeel Butt]

On Thu, Aug 20, 2020 at 6:04 AM Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> The swap page counter is v2 only while memsw is v1 only. As v1 and v2
> controllers cannot be active at the same time, there is no point to keep
> both swap and memsw page counters in mem_cgroup. The previous patch has
> made sure that memsw page counter is updated and accessed only when in
> v1 code paths. So it is now safe to alias the v1 memsw page counter to v2
> swap page counter. This saves 14 long's in the size of mem_cgroup. This
> is a saving of 112 bytes for 64-bit archs.
>
> Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  include/linux/memcontrol.h | 3 +--
>  mm/memcontrol.c            | 8 +++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index d0b036123c6a..d2a819d7db70 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -216,10 +216,9 @@ struct mem_cgroup {
>
>         /* Accounted resources */
>         struct page_counter memory;
> -       struct page_counter swap;
> +       struct page_counter swap;       /* memsw (memory+swap) for v1 */
>
>         /* Legacy consumer-oriented counters */
> -       struct page_counter memsw;
>         struct page_counter kmem;
>         struct page_counter tcpmem;
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index d219dca5239f..04c3794cdc98 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -68,6 +68,11 @@
>
>  #include <trace/events/vmscan.h>
>
> +/*
> + * The v1 memsw page counter is aliased to the v2 swap page counter.
> + */
> +#define memsw  swap
> +

Personally I would prefer a union instead of #define.

>  struct cgroup_subsys memory_cgrp_subsys __read_mostly;
>  EXPORT_SYMBOL(memory_cgrp_subsys);
>
> @@ -5279,13 +5284,11 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
>                 memcg->use_hierarchy = true;
>                 page_counter_init(&memcg->memory, &parent->memory);
>                 page_counter_init(&memcg->swap, &parent->swap);
> -               page_counter_init(&memcg->memsw, &parent->memsw);
>                 page_counter_init(&memcg->kmem, &parent->kmem);
>                 page_counter_init(&memcg->tcpmem, &parent->tcpmem);
>         } else {
>                 page_counter_init(&memcg->memory, NULL);
>                 page_counter_init(&memcg->swap, NULL);
> -               page_counter_init(&memcg->memsw, NULL);
>                 page_counter_init(&memcg->kmem, NULL);
>                 page_counter_init(&memcg->tcpmem, NULL);
>                 /*
> @@ -5414,7 +5417,6 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css)
>
>         page_counter_set_max(&memcg->memory, PAGE_COUNTER_MAX);
>         page_counter_set_max(&memcg->swap, PAGE_COUNTER_MAX);
> -       page_counter_set_max(&memcg->memsw, PAGE_COUNTER_MAX);
>         page_counter_set_max(&memcg->kmem, PAGE_COUNTER_MAX);
>         page_counter_set_max(&memcg->tcpmem, PAGE_COUNTER_MAX);
>         page_counter_set_min(&memcg->memory, 0);
> --
> 2.18.1
>

Re: [PATCH 3/3] mm/memcg: Unify swap and memsw page counters

[Waiman Long]

On 8/20/20 11:46 AM, Shakeel Butt wrote:
> On Thu, Aug 20, 2020 at 6:04 AM Waiman Long <longman@redhat.com> wrote:
>> The swap page counter is v2 only while memsw is v1 only. As v1 and v2
>> controllers cannot be active at the same time, there is no point to keep
>> both swap and memsw page counters in mem_cgroup. The previous patch has
>> made sure that memsw page counter is updated and accessed only when in
>> v1 code paths. So it is now safe to alias the v1 memsw page counter to v2
>> swap page counter. This saves 14 long's in the size of mem_cgroup. This
>> is a saving of 112 bytes for 64-bit archs.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   include/linux/memcontrol.h | 3 +--
>>   mm/memcontrol.c            | 8 +++++---
>>   2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index d0b036123c6a..d2a819d7db70 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -216,10 +216,9 @@ struct mem_cgroup {
>>
>>          /* Accounted resources */
>>          struct page_counter memory;
>> -       struct page_counter swap;
>> +       struct page_counter swap;       /* memsw (memory+swap) for v1 */
>>
>>          /* Legacy consumer-oriented counters */
>> -       struct page_counter memsw;
>>          struct page_counter kmem;
>>          struct page_counter tcpmem;
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index d219dca5239f..04c3794cdc98 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -68,6 +68,11 @@
>>
>>   #include <trace/events/vmscan.h>
>>
>> +/*
>> + * The v1 memsw page counter is aliased to the v2 swap page counter.
>> + */
>> +#define memsw  swap
>> +
> Personally I would prefer a union instead of #define.

Yes, that is also what I am thinking about in the v2.

Cheers,
Longman