Re: [v2 PATCH 8/9] mm: memcontrol: reparent nr_deferred when memcg offline

2020-12-15 Thread Yang Shi
On Mon, Dec 14, 2020 at 7:08 PM Dave Chinner  wrote:
>
> On Mon, Dec 14, 2020 at 02:37:21PM -0800, Yang Shi wrote:
> > Now shrinker's nr_deferred is per memcg for memcg aware shrinkers, add to 
> > parent's
> > corresponding nr_deferred when memcg offline.
> >
> > Signed-off-by: Yang Shi 
> > ---
> >  include/linux/shrinker.h |  4 
> >  mm/memcontrol.c  | 24 
> >  mm/vmscan.c  |  2 +-
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> > index 1eac79ce57d4..85cfc910dde4 100644
> > --- a/include/linux/shrinker.h
> > +++ b/include/linux/shrinker.h
> > @@ -78,6 +78,10 @@ struct shrinker {
> >  };
> >  #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */
> >
> > +#ifdef CONFIG_MEMCG
> > +extern int shrinker_nr_max;
> > +#endif
> > +
> >  /* Flags */
> >  #define SHRINKER_REGISTERED  (1 << 0)
> >  #define SHRINKER_NUMA_AWARE  (1 << 1)
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 321d1818ce3d..1f191a15bee1 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -59,6 +59,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include "internal.h"
> >  #include 
> >  #include 
> > @@ -612,6 +613,28 @@ void memcg_set_shrinker_bit(struct mem_cgroup *memcg, 
> > int nid, int shrinker_id)
> >   }
> >  }
> >
> > +static void memcg_reparent_shrinker_deferred(struct mem_cgroup *memcg)
> > +{
> > + int i, nid;
> > + long nr;
> > + struct mem_cgroup *parent;
> > + struct memcg_shrinker_deferred *child_deferred, *parent_deferred;
> > +
> > + parent = parent_mem_cgroup(memcg);
> > + if (!parent)
> > + parent = root_mem_cgroup;
> > +
> > + for_each_node(nid) {
> > + child_deferred = memcg->nodeinfo[nid]->shrinker_deferred;
> > + parent_deferred = parent->nodeinfo[nid]->shrinker_deferred;
> > + for (i = 0; i < shrinker_nr_max; i ++) {
> > + nr = 
> > atomic_long_read(_deferred->nr_deferred[i]);
> > + atomic_long_add(nr,
> > + _deferred->nr_deferred[i]);
> > + }
> > + }
> > +}
>
> I would place this function in vmscan.c alongside the
> shrink_slab_set_nr_deferred_memcg() function so that all the
> accounting is in the one place.

Fine to me. Will incorporate in v3.

>
> > +
> >  /**
> >   * mem_cgroup_css_from_page - css of the memcg associated with a page
> >   * @page: page of interest
> > @@ -5543,6 +5566,7 @@ static void mem_cgroup_css_offline(struct 
> > cgroup_subsys_state *css)
> >   page_counter_set_low(>memory, 0);
> >
> >   memcg_offline_kmem(memcg);
> > + memcg_reparent_shrinker_deferred(memcg);
> >   wb_memcg_offline(memcg);
> >
> >   drain_all_stock(memcg);
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 8d5bfd818acd..693a41e89969 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -201,7 +201,7 @@ DECLARE_RWSEM(shrinker_rwsem);
> >  #define SHRINKER_REGISTERING ((struct shrinker *)~0UL)
> >
> >  static DEFINE_IDR(shrinker_idr);
> > -static int shrinker_nr_max;
> > +int shrinker_nr_max;
>
> Then we don't need to make yet another variable global...
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> da...@fromorbit.com


Re: [v2 PATCH 8/9] mm: memcontrol: reparent nr_deferred when memcg offline

2020-12-14 Thread Dave Chinner
On Mon, Dec 14, 2020 at 02:37:21PM -0800, Yang Shi wrote:
> Now shrinker's nr_deferred is per memcg for memcg aware shrinkers, add to 
> parent's
> corresponding nr_deferred when memcg offline.
> 
> Signed-off-by: Yang Shi 
> ---
>  include/linux/shrinker.h |  4 
>  mm/memcontrol.c  | 24 
>  mm/vmscan.c  |  2 +-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> index 1eac79ce57d4..85cfc910dde4 100644
> --- a/include/linux/shrinker.h
> +++ b/include/linux/shrinker.h
> @@ -78,6 +78,10 @@ struct shrinker {
>  };
>  #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */
>  
> +#ifdef CONFIG_MEMCG
> +extern int shrinker_nr_max;
> +#endif
> +
>  /* Flags */
>  #define SHRINKER_REGISTERED  (1 << 0)
>  #define SHRINKER_NUMA_AWARE  (1 << 1)
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 321d1818ce3d..1f191a15bee1 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -59,6 +59,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "internal.h"
>  #include 
>  #include 
> @@ -612,6 +613,28 @@ void memcg_set_shrinker_bit(struct mem_cgroup *memcg, 
> int nid, int shrinker_id)
>   }
>  }
>  
> +static void memcg_reparent_shrinker_deferred(struct mem_cgroup *memcg)
> +{
> + int i, nid;
> + long nr;
> + struct mem_cgroup *parent;
> + struct memcg_shrinker_deferred *child_deferred, *parent_deferred;
> +
> + parent = parent_mem_cgroup(memcg);
> + if (!parent)
> + parent = root_mem_cgroup;
> +
> + for_each_node(nid) {
> + child_deferred = memcg->nodeinfo[nid]->shrinker_deferred;
> + parent_deferred = parent->nodeinfo[nid]->shrinker_deferred;
> + for (i = 0; i < shrinker_nr_max; i ++) {
> + nr = atomic_long_read(_deferred->nr_deferred[i]);
> + atomic_long_add(nr,
> + _deferred->nr_deferred[i]);
> + }
> + }
> +}

I would place this function in vmscan.c alongside the
shrink_slab_set_nr_deferred_memcg() function so that all the
accounting is in the one place.

> +
>  /**
>   * mem_cgroup_css_from_page - css of the memcg associated with a page
>   * @page: page of interest
> @@ -5543,6 +5566,7 @@ static void mem_cgroup_css_offline(struct 
> cgroup_subsys_state *css)
>   page_counter_set_low(>memory, 0);
>  
>   memcg_offline_kmem(memcg);
> + memcg_reparent_shrinker_deferred(memcg);
>   wb_memcg_offline(memcg);
>  
>   drain_all_stock(memcg);
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 8d5bfd818acd..693a41e89969 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -201,7 +201,7 @@ DECLARE_RWSEM(shrinker_rwsem);
>  #define SHRINKER_REGISTERING ((struct shrinker *)~0UL)
>  
>  static DEFINE_IDR(shrinker_idr);
> -static int shrinker_nr_max;
> +int shrinker_nr_max;

Then we don't need to make yet another variable global...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com


[v2 PATCH 8/9] mm: memcontrol: reparent nr_deferred when memcg offline

2020-12-14 Thread Yang Shi
Now shrinker's nr_deferred is per memcg for memcg aware shrinkers, add to 
parent's
corresponding nr_deferred when memcg offline.

Signed-off-by: Yang Shi 
---
 include/linux/shrinker.h |  4 
 mm/memcontrol.c  | 24 
 mm/vmscan.c  |  2 +-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 1eac79ce57d4..85cfc910dde4 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -78,6 +78,10 @@ struct shrinker {
 };
 #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */
 
+#ifdef CONFIG_MEMCG
+extern int shrinker_nr_max;
+#endif
+
 /* Flags */
 #define SHRINKER_REGISTERED(1 << 0)
 #define SHRINKER_NUMA_AWARE(1 << 1)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 321d1818ce3d..1f191a15bee1 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "internal.h"
 #include 
 #include 
@@ -612,6 +613,28 @@ void memcg_set_shrinker_bit(struct mem_cgroup *memcg, int 
nid, int shrinker_id)
}
 }
 
+static void memcg_reparent_shrinker_deferred(struct mem_cgroup *memcg)
+{
+   int i, nid;
+   long nr;
+   struct mem_cgroup *parent;
+   struct memcg_shrinker_deferred *child_deferred, *parent_deferred;
+
+   parent = parent_mem_cgroup(memcg);
+   if (!parent)
+   parent = root_mem_cgroup;
+
+   for_each_node(nid) {
+   child_deferred = memcg->nodeinfo[nid]->shrinker_deferred;
+   parent_deferred = parent->nodeinfo[nid]->shrinker_deferred;
+   for (i = 0; i < shrinker_nr_max; i ++) {
+   nr = atomic_long_read(_deferred->nr_deferred[i]);
+   atomic_long_add(nr,
+   _deferred->nr_deferred[i]);
+   }
+   }
+}
+
 /**
  * mem_cgroup_css_from_page - css of the memcg associated with a page
  * @page: page of interest
@@ -5543,6 +5566,7 @@ static void mem_cgroup_css_offline(struct 
cgroup_subsys_state *css)
page_counter_set_low(>memory, 0);
 
memcg_offline_kmem(memcg);
+   memcg_reparent_shrinker_deferred(memcg);
wb_memcg_offline(memcg);
 
drain_all_stock(memcg);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 8d5bfd818acd..693a41e89969 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -201,7 +201,7 @@ DECLARE_RWSEM(shrinker_rwsem);
 #define SHRINKER_REGISTERING ((struct shrinker *)~0UL)
 
 static DEFINE_IDR(shrinker_idr);
-static int shrinker_nr_max;
+int shrinker_nr_max;
 
 static inline bool is_deferred_memcg_aware(struct shrinker *shrinker)
 {
-- 
2.26.2