Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
the pagefault and exit_mmap paths when memcgs are disabled using
cgroup_disable=memory command-line option.
This change results in ~2.1% overhead reduction when running PFT test
comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
ARM64 Android device.
Signed-off-by: Suren Baghdasaryan <[email protected]>
Reviewed-by: Shakeel Butt <[email protected]>
Acked-by: Johannes Weiner <[email protected]>
---
mm/memcontrol.c | 3 +++
mm/swapfile.c | 3 +++
mm/vmpressure.c | 7 ++++++-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ae1f5d0cb581..a228cd51c4bd 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
struct mem_cgroup *memcg;
unsigned short id;
+ if (mem_cgroup_disabled())
+ return;
+
id = swap_cgroup_record(entry, 0, nr_pages);
rcu_read_lock();
memcg = mem_cgroup_from_id(id);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 1e07d1c776f2..707fa0481bb4 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
struct swap_info_struct *si, *next;
int nid = page_to_nid(page);
+ if (mem_cgroup_disabled())
+ return;
+
if (!(gfp_mask & __GFP_IO))
return;
diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index d69019fc3789..9b172561fded 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
unsigned long scanned, unsigned long reclaimed)
{
- struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
+ struct vmpressure *vmpr;
+
+ if (mem_cgroup_disabled())
+ return;
+
+ vmpr = memcg_to_vmpressure(memcg);
/*
* Here we only want to account pressure that userland is able to
--
2.32.0.93.g670b81a890-goog
On 2021/7/10 8:36, Suren Baghdasaryan wrote:
> Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
> cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
> the pagefault and exit_mmap paths when memcgs are disabled using
> cgroup_disable=memory command-line option.
> This change results in ~2.1% overhead reduction when running PFT test
> comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
> CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
> ARM64 Android device.
>
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> Reviewed-by: Shakeel Butt <[email protected]>
> Acked-by: Johannes Weiner <[email protected]>
> ---
> mm/memcontrol.c | 3 +++
> mm/swapfile.c | 3 +++
> mm/vmpressure.c | 7 ++++++-
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ae1f5d0cb581..a228cd51c4bd 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> struct mem_cgroup *memcg;
> unsigned short id;
>
> + if (mem_cgroup_disabled())
> + return;
> +
> id = swap_cgroup_record(entry, 0, nr_pages);
> rcu_read_lock();
> memcg = mem_cgroup_from_id(id);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 1e07d1c776f2..707fa0481bb4 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
> struct swap_info_struct *si, *next;
> int nid = page_to_nid(page);
>
> + if (mem_cgroup_disabled())
> + return;
> +
Many thanks for your patch. But I'am somewhat confused about this change.
IMO, cgroup_throttle_swaprate() is only related to blk_cgroup and it seems
it's irrelevant to mem_cgroup. Could you please have a explanation for me?
Thanks!
> if (!(gfp_mask & __GFP_IO))
> return;
>
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> index d69019fc3789..9b172561fded 100644
> --- a/mm/vmpressure.c
> +++ b/mm/vmpressure.c
> @@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
> void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
> unsigned long scanned, unsigned long reclaimed)
> {
> - struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
> + struct vmpressure *vmpr;
> +
> + if (mem_cgroup_disabled())
> + return;
> +
> + vmpr = memcg_to_vmpressure(memcg);
>
> /*
> * Here we only want to account pressure that userland is able to
>
On Fri, Jul 9, 2021 at 6:52 PM Miaohe Lin <[email protected]> wrote:
>
> On 2021/7/10 8:36, Suren Baghdasaryan wrote:
> > Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
> > cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
> > the pagefault and exit_mmap paths when memcgs are disabled using
> > cgroup_disable=memory command-line option.
> > This change results in ~2.1% overhead reduction when running PFT test
> > comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
> > CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
> > ARM64 Android device.
> >
> > Signed-off-by: Suren Baghdasaryan <[email protected]>
> > Reviewed-by: Shakeel Butt <[email protected]>
> > Acked-by: Johannes Weiner <[email protected]>
> > ---
> > mm/memcontrol.c | 3 +++
> > mm/swapfile.c | 3 +++
> > mm/vmpressure.c | 7 ++++++-
> > 3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index ae1f5d0cb581..a228cd51c4bd 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> > struct mem_cgroup *memcg;
> > unsigned short id;
> >
> > + if (mem_cgroup_disabled())
> > + return;
> > +
> > id = swap_cgroup_record(entry, 0, nr_pages);
> > rcu_read_lock();
> > memcg = mem_cgroup_from_id(id);
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index 1e07d1c776f2..707fa0481bb4 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
> > struct swap_info_struct *si, *next;
> > int nid = page_to_nid(page);
> >
> > + if (mem_cgroup_disabled())
> > + return;
> > +
>
> Many thanks for your patch. But I'am somewhat confused about this change.
> IMO, cgroup_throttle_swaprate() is only related to blk_cgroup and it seems
> it's irrelevant to mem_cgroup. Could you please have a explanation for me?
cgroup_throttle_swaprate() is a NoOp when CONFIG_MEMCG=n (see:
https://elixir.bootlin.com/linux/latest/source/include/linux/swap.h#L699),
therefore I assume we can safely skip it when memcgs are disabled via
"cgroup_disable=memory". From perf results I also see no hits on this
function when CONFIG_MEMCG=n.
However, looking into the code, I'm not sure why it should depend on
CONFIG_MEMCG. But it's Friday night and I might be missing some
details here...
>
> Thanks!
>
> > if (!(gfp_mask & __GFP_IO))
> > return;
> >
> > diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> > index d69019fc3789..9b172561fded 100644
> > --- a/mm/vmpressure.c
> > +++ b/mm/vmpressure.c
> > @@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
> > void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
> > unsigned long scanned, unsigned long reclaimed)
> > {
> > - struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
> > + struct vmpressure *vmpr;
> > +
> > + if (mem_cgroup_disabled())
> > + return;
> > +
> > + vmpr = memcg_to_vmpressure(memcg);
> >
> > /*
> > * Here we only want to account pressure that userland is able to
> >
>
On 2021/7/10 10:40, Suren Baghdasaryan wrote:
> On Fri, Jul 9, 2021 at 6:52 PM Miaohe Lin <[email protected]> wrote:
>>
>> On 2021/7/10 8:36, Suren Baghdasaryan wrote:
>>> Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
>>> cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
>>> the pagefault and exit_mmap paths when memcgs are disabled using
>>> cgroup_disable=memory command-line option.
>>> This change results in ~2.1% overhead reduction when running PFT test
>>> comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
>>> CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
>>> ARM64 Android device.
>>>
>>> Signed-off-by: Suren Baghdasaryan <[email protected]>
>>> Reviewed-by: Shakeel Butt <[email protected]>
>>> Acked-by: Johannes Weiner <[email protected]>
>>> ---
>>> mm/memcontrol.c | 3 +++
>>> mm/swapfile.c | 3 +++
>>> mm/vmpressure.c | 7 ++++++-
>>> 3 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index ae1f5d0cb581..a228cd51c4bd 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
>>> struct mem_cgroup *memcg;
>>> unsigned short id;
>>>
>>> + if (mem_cgroup_disabled())
>>> + return;
>>> +
>>> id = swap_cgroup_record(entry, 0, nr_pages);
>>> rcu_read_lock();
>>> memcg = mem_cgroup_from_id(id);
>>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>>> index 1e07d1c776f2..707fa0481bb4 100644
>>> --- a/mm/swapfile.c
>>> +++ b/mm/swapfile.c
>>> @@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
>>> struct swap_info_struct *si, *next;
>>> int nid = page_to_nid(page);
>>>
>>> + if (mem_cgroup_disabled())
>>> + return;
>>> +
>>
>> Many thanks for your patch. But I'am somewhat confused about this change.
>> IMO, cgroup_throttle_swaprate() is only related to blk_cgroup and it seems
>> it's irrelevant to mem_cgroup. Could you please have a explanation for me?
>
> cgroup_throttle_swaprate() is a NoOp when CONFIG_MEMCG=n (see:
> https://elixir.bootlin.com/linux/latest/source/include/linux/swap.h#L699),
I browsed the git history related to cgroup_throttle_swaprate() and found this:
"""
mm: memcontrol: move out cgroup swaprate throttling
The cgroup swaprate throttling is about matching new anon allocations to
the rate of available IO when that is being throttled. It's the io
controller hooking into the VM, rather than a memory controller thing.
"""
It seems cgroup_throttle_swaprate() is working with memory allocations.
So mem_cgroup matters this way. But I'am not sure...
> therefore I assume we can safely skip it when memcgs are disabled via
> "cgroup_disable=memory". From perf results I also see no hits on this
> function when CONFIG_MEMCG=n.
> However, looking into the code, I'm not sure why it should depend on
> CONFIG_MEMCG. But it's Friday night and I might be missing some
> details here...
Many thanks for your replay at Friday night. :)
>
>>
>> Thanks!
>>
>>> if (!(gfp_mask & __GFP_IO))
>>> return;
>>>
>>> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
>>> index d69019fc3789..9b172561fded 100644
>>> --- a/mm/vmpressure.c
>>> +++ b/mm/vmpressure.c
>>> @@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
>>> void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
>>> unsigned long scanned, unsigned long reclaimed)
>>> {
>>> - struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
>>> + struct vmpressure *vmpr;
>>> +
>>> + if (mem_cgroup_disabled())
>>> + return;
>>> +
>>> + vmpr = memcg_to_vmpressure(memcg);
>>>
>>> /*
>>> * Here we only want to account pressure that userland is able to
>>>
>>
> .
>
On Sat, Jul 10, 2021 at 8:36 AM Suren Baghdasaryan <[email protected]> wrote:
>
> Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
> cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
> the pagefault and exit_mmap paths when memcgs are disabled using
> cgroup_disable=memory command-line option.
> This change results in ~2.1% overhead reduction when running PFT test
> comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
> CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
> ARM64 Android device.
>
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> Reviewed-by: Shakeel Butt <[email protected]>
> Acked-by: Johannes Weiner <[email protected]>
The changes are straightforward. LGTM.
Reviewed-by: Muchun Song <[email protected]>
On Fri 09-07-21 17:36:24, Suren Baghdasaryan wrote:
> Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
> cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
> the pagefault and exit_mmap paths when memcgs are disabled using
> cgroup_disable=memory command-line option.
> This change results in ~2.1% overhead reduction when running PFT test
What is PFT test?
> comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
> CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
> ARM64 Android device.
>
> Signed-off-by: Suren Baghdasaryan <[email protected]>
> Reviewed-by: Shakeel Butt <[email protected]>
> Acked-by: Johannes Weiner <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Thanks!
> ---
> mm/memcontrol.c | 3 +++
> mm/swapfile.c | 3 +++
> mm/vmpressure.c | 7 ++++++-
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ae1f5d0cb581..a228cd51c4bd 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> struct mem_cgroup *memcg;
> unsigned short id;
>
> + if (mem_cgroup_disabled())
> + return;
> +
> id = swap_cgroup_record(entry, 0, nr_pages);
> rcu_read_lock();
> memcg = mem_cgroup_from_id(id);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 1e07d1c776f2..707fa0481bb4 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
> struct swap_info_struct *si, *next;
> int nid = page_to_nid(page);
>
> + if (mem_cgroup_disabled())
> + return;
> +
> if (!(gfp_mask & __GFP_IO))
> return;
>
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> index d69019fc3789..9b172561fded 100644
> --- a/mm/vmpressure.c
> +++ b/mm/vmpressure.c
> @@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
> void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
> unsigned long scanned, unsigned long reclaimed)
> {
> - struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
> + struct vmpressure *vmpr;
> +
> + if (mem_cgroup_disabled())
> + return;
> +
> + vmpr = memcg_to_vmpressure(memcg);
>
> /*
> * Here we only want to account pressure that userland is able to
> --
> 2.32.0.93.g670b81a890-goog
--
Michal Hocko
SUSE Labs
On Mon, Jul 12, 2021 at 12:11 AM Michal Hocko <[email protected]> wrote:
>
> On Fri 09-07-21 17:36:24, Suren Baghdasaryan wrote:
> > Add mem_cgroup_disabled check in vmpressure, mem_cgroup_uncharge_swap and
> > cgroup_throttle_swaprate functions. This minimizes the memcg overhead in
> > the pagefault and exit_mmap paths when memcgs are disabled using
> > cgroup_disable=memory command-line option.
> > This change results in ~2.1% overhead reduction when running PFT test
>
> What is PFT test?
Christoph Lamenter’s pagefault tool
(https://lkml.org/lkml/2006/8/29/294). I'll add the link in the
description for clarity.
>
> > comparing {CONFIG_MEMCG=n, CONFIG_MEMCG_SWAP=n} against {CONFIG_MEMCG=y,
> > CONFIG_MEMCG_SWAP=y, cgroup_disable=memory} configuration on an 8-core
> > ARM64 Android device.
> >
> > Signed-off-by: Suren Baghdasaryan <[email protected]>
> > Reviewed-by: Shakeel Butt <[email protected]>
> > Acked-by: Johannes Weiner <[email protected]>
>
> Acked-by: Michal Hocko <[email protected]>
Thanks!
>
> Thanks!
>
> > ---
> > mm/memcontrol.c | 3 +++
> > mm/swapfile.c | 3 +++
> > mm/vmpressure.c | 7 ++++++-
> > 3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index ae1f5d0cb581..a228cd51c4bd 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -7305,6 +7305,9 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
> > struct mem_cgroup *memcg;
> > unsigned short id;
> >
> > + if (mem_cgroup_disabled())
> > + return;
> > +
> > id = swap_cgroup_record(entry, 0, nr_pages);
> > rcu_read_lock();
> > memcg = mem_cgroup_from_id(id);
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index 1e07d1c776f2..707fa0481bb4 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -3778,6 +3778,9 @@ void cgroup_throttle_swaprate(struct page *page, gfp_t gfp_mask)
> > struct swap_info_struct *si, *next;
> > int nid = page_to_nid(page);
> >
> > + if (mem_cgroup_disabled())
> > + return;
> > +
> > if (!(gfp_mask & __GFP_IO))
> > return;
> >
> > diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> > index d69019fc3789..9b172561fded 100644
> > --- a/mm/vmpressure.c
> > +++ b/mm/vmpressure.c
> > @@ -240,7 +240,12 @@ static void vmpressure_work_fn(struct work_struct *work)
> > void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
> > unsigned long scanned, unsigned long reclaimed)
> > {
> > - struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
> > + struct vmpressure *vmpr;
> > +
> > + if (mem_cgroup_disabled())
> > + return;
> > +
> > + vmpr = memcg_to_vmpressure(memcg);
> >
> > /*
> > * Here we only want to account pressure that userland is able to
> > --
> > 2.32.0.93.g670b81a890-goog
>
> --
> Michal Hocko
> SUSE Labs