Re: [PATCH] memcg: force charge kmem counter too

[Michal Hocko]

On Thu 31-05-18 17:23:17, Minchan Kim wrote:
> On Thu, May 31, 2018 at 08:56:42AM +0200, Michal Hocko wrote:
> > On Thu 31-05-18 15:01:33, Minchan Kim wrote:
> > > On Wed, May 30, 2018 at 11:14:33AM -0700, Shakeel Butt wrote:
> > > > On Tue, May 29, 2018 at 1:31 AM, Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> > > > > On Mon 28-05-18 10:23:07, Shakeel Butt wrote:
> > > > >> On Mon, May 28, 2018 at 2:11 AM, Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> > > > >> Though is there a precedence where the broken feature is not fixed
> > > > >> because an alternative is available?
> > > > >
> > > > > Well, I can see how breaking GFP_NOFAIL semantic is problematic, on the
> > > > > other hand we keep saying that kmem accounting in v1 is hard usable and
> > > > > strongly discourage people from using it. Sure we can add the code which
> > > > > handles _this_ particular case but that wouldn't make the whole thing
> > > > > more usable I strongly suspect. Maybe I am wrong and you can provide
> > > > > some specific examples. Is GFP_NOFAIL that common to matter?
> > > > >
> > > > > In any case we should balance between the code maintainability here.
> > > > > Adding more cruft into the allocator path is not free.
> > > > >
> > > > 
> > > > We do not use kmem limits internally and this is something I found
> > > > through code inspection. If this patch is increasing the cost of code
> > > > maintainability I am fine with dropping it but at least there should a
> > > > comment saying that kmem limits are broken and no need fix.
> > >  
> > > I agree.
> > > 
> > > Even, I didn't know kmem is strongly discouraged until now. Then,
> > > why is it enabled by default on cgroup v1?
> > 
> > You have to set a non-zero limit to make it active IIRC. The code is
> 
> Maybe, no. I didn't set anything. IOW, it was a default without any setting
> and I hit this as you can remember.
> http://lkml.kernel.org/r/<20180418022912.248417-1-minchan@xxxxxxxxxx>
> We don't need to allocate memory for stuff even maintainers discourage.

Well, you can disable the whole kmem accounting by cgroup.memory=nokmem
kernel parameter. I wasn't very happy when we removed the config option
which would have put all the tracking aside even when no limit is set
but the argument was that the ifdefery was growing into an
unmaintainable mess and I have to agree with that. So here we are...
-- 
Michal Hocko
SUSE Labs