lkml.org 
[lkml]   [2021]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [v3 PATCH 10/11] mm: memcontrol: reparent nr_deferred when memcg offline
    On Wed, Jan 6, 2021 at 3:35 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
    >
    > On 06.01.2021 01:58, 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 <shy828301@gmail.com>
    > > ---
    > > include/linux/memcontrol.h | 1 +
    > > mm/memcontrol.c | 1 +
    > > mm/vmscan.c | 29 +++++++++++++++++++++++++++++
    > > 3 files changed, 31 insertions(+)
    > >
    > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
    > > index 5599082df623..d1e52e916cc2 100644
    > > --- a/include/linux/memcontrol.h
    > > +++ b/include/linux/memcontrol.h
    > > @@ -1586,6 +1586,7 @@ extern int memcg_alloc_shrinker_info(struct mem_cgroup *memcg);
    > > extern void memcg_free_shrinker_info(struct mem_cgroup *memcg);
    > > extern void memcg_set_shrinker_bit(struct mem_cgroup *memcg,
    > > int nid, int shrinker_id);
    > > +extern void memcg_reparent_shrinker_deferred(struct mem_cgroup *memcg);
    > > #else
    > > #define mem_cgroup_sockets_enabled 0
    > > static inline void mem_cgroup_sk_alloc(struct sock *sk) { };
    > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    > > index 126f1fd550c8..19e555675582 100644
    > > --- a/mm/memcontrol.c
    > > +++ b/mm/memcontrol.c
    > > @@ -5284,6 +5284,7 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
    > > page_counter_set_low(&memcg->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 d9795fb0f1c5..71056057d26d 100644
    > > --- a/mm/vmscan.c
    > > +++ b/mm/vmscan.c
    > > @@ -396,6 +396,35 @@ static long set_nr_deferred_memcg(long nr, int nid, struct shrinker *shrinker,
    > > return atomic_long_add_return(nr, &info->nr_deferred[shrinker->id]);
    > > }
    > >
    > > +void memcg_reparent_shrinker_deferred(struct mem_cgroup *memcg)
    > > +{
    > > + int i, nid;
    > > + long nr;
    > > + struct mem_cgroup *parent;
    > > + struct memcg_shrinker_info *child_info, *parent_info;
    > > +
    > > + parent = parent_mem_cgroup(memcg);
    > > + if (!parent)
    > > + parent = root_mem_cgroup;
    > > +
    > > + /* Prevent from concurrent shrinker_info expand */
    > > + down_read(&shrinker_rwsem);
    > > + for_each_node(nid) {
    > > + child_info = rcu_dereference_protected(
    > > + memcg->nodeinfo[nid]->shrinker_info,
    > > + true);
    > > + parent_info = rcu_dereference_protected(
    > > + parent->nodeinfo[nid]->shrinker_info,
    > > + true);
    >
    > Simple assignment can't take such lots of space, we have to do something with that.
    >
    > Number of these
    >
    > rcu_dereference_protected(memcg->nodeinfo[nid]->shrinker_info, true)
    >
    > became too big, and we can't allow every of them takes 3 lines.
    >
    > We should introduce a short helper to dereferrence this, so we will be able to give
    > out attention to really difficult logic instead of wasting it on parsing this.
    >
    > child_info = memcg_shrinker_info(memcg, nid);
    > or
    > child_info = memcg_shrinker_info_protected(memcg, nid);
    >
    > Both of them fit in single line.
    >
    > struct memcg_shrinker_info *memcg_shrinker_info_protected(
    > struct mem_cgroup *memcg, int nid)
    > {
    > return rcu_dereference_protected(memcg->nodeinfo[nid]->shrinker_info,
    > lockdep_assert_held(&shrinker_rwsem));
    > }

    Thanks for the suggestion, it makes sense to me. Will incorporate it in v4.

    >
    >
    > > + for (i = 0; i < shrinker_nr_max; i++) {
    > > + nr = atomic_long_read(&child_info->nr_deferred[i]);
    > > + atomic_long_add(nr,
    > > + &parent_info->nr_deferred[i]);
    >
    > Why new line is here? In case of you merge it up, it will be even shorter then previous line.

    Just keep in 80 lines. We could relax it.

    >
    > > + }
    > > + }
    > > + up_read(&shrinker_rwsem);
    > > +}
    > > +
    > > static bool cgroup_reclaim(struct scan_control *sc)
    > > {
    > > return sc->target_mem_cgroup;
    > >
    >
    >

    \
     
     \ /
      Last update: 2021-01-11 19:45    [W:3.647 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site