Re: [PATCH 1/2] memcg: folding CONFIG_MEMCG_SWAP as default

[Shakeel Butt <shakeelb@google.com>]

On Sat, Apr 18, 2020 at 6:45 AM Alex Shi <alex.shi@linux.alibaba.com> wrote:
>
>
>
> 在 2020/4/18 上午12:54, Michal Hocko 写道:
> > On Fri 17-04-20 09:41:04, Shakeel Butt wrote:
> >> On Fri, Apr 17, 2020 at 9:03 AM Michal Hocko <mhocko@kernel.org> wrote:
> >>>
> >>> On Fri 17-04-20 22:43:43, Alex Shi wrote:
> >>>> This patch fold MEMCG_SWAP feature into kernel as default function. That
> >>>> required a short size memcg id for each of page. As Johannes mentioned
> >>>>
> >>>> "the overhead of tracking is tiny - 512k per G of swap (0.04%).'
> >>>>
> >>>> So all swapout page could be tracked for its memcg id.
> >>>
> >>> I am perfectly OK with dropping the CONFIG_MEMCG_SWAP. The code that is
> >>> guarded by it is negligible and the resulting code is much easier to
> >>> read so no objection on that front. I just do not really see any real
> >>> reason to flip the default for cgroup v1. Why do we want/need that?
> >>>
> >>
> >> Yes, the changelog is lacking the motivation of this change. This is
> >> proposed by Johannes and I was actually expecting the patch from him.
> >> The motivation is to make the things simpler for per-memcg LRU locking
> >> and workingset for anon memory (Johannes has described these really
> >> well, lemme find the email). If we keep the differentiation between
> >> cgroup v1 and v2, then there is actually no point of this cleanup as
> >> per-memcg LRU locking and anon workingset still has to handle the
> >> !do_swap_account case.
> >
> > All those details really have to go into the changelog. I have to say
> > that I still do not understand why the actual accounting swap or not
> > makes any difference for per per-memcg LRU. Especially when your patch
> > keeps the kernel command line parameter still in place.
> >
> > Anyway, it would be much more simpler to have a patch that drops the
> > CONFIG_MEMCG_SWAP and a separate one which switches the default
> > beahvior. I am not saying I am ok with the later but if the
> > justification is convincing then I might change my mind.
> >
>
> Hi Shakeel & Michal,
>
> Thanks for all comments!
>
> Yes, we still need to remove swapaccount from cmdline and keep swap_cgroup.id
> permanently. Just I don't know if this patch could fit into the details of
> Johannes new solution.
>
> Anyway, I will send out v2 for complete memcg id record patch, just in case
> if they are useful.
>

I would recommend waiting for Johannes patch series. The cleanup this
patch is doing makes more sense to be part of Johannes's lrucare
cleanup series.

thanks,
Shakeel