linux-kernel - Re: [PATCH 09/16] mm: memcg/slab: charge individual slab objects instead of pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 31 Oct 2019 10:41:51 -0400
From:   Johannes Weiner <hannes@...xchg.org>
To:     Roman Gushchin <guro@...com>
Cc:     "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Michal Hocko <mhocko@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Waiman Long <longman@...hat.com>,
        Christoph Lameter <cl@...ux.com>
Subject: Re: [PATCH 09/16] mm: memcg/slab: charge individual slab objects
 instead of pages

On Thu, Oct 31, 2019 at 01:52:44AM +0000, Roman Gushchin wrote:
> On Fri, Oct 25, 2019 at 03:41:18PM -0400, Johannes Weiner wrote:
> > @@ -3117,15 +3095,24 @@ void __memcg_kmem_uncharge(struct page *page, int order)
> >  	css_put_many(&memcg->css, nr_pages);
> >  }
> >  
> > -int __memcg_kmem_charge_subpage(struct mem_cgroup *memcg, size_t size,
> > -				gfp_t gfp)
> > +int obj_cgroup_charge(struct obj_cgroup *objcg, size_t size, gfp_t gfp)
> >  {
> > -	return try_charge(memcg, gfp, size, true);
> > +	int ret;
> > +
> > +	if (consume_obj_stock(objcg, nr_bytes))
> > +		return 0;
> > +
> > +	ret = try_charge(objcg->memcg, gfp, 1);
> > +	if (ret)
> > +		return ret;

> The second problem is also here. If a task belonging to a different memcg
> is scheduled on this cpu, most likely we will need to refill both stocks,
> even if we need only a small temporarily allocation.

Yes, that's a good thing. The reason we have the per-cpu caches in the
first place is because most likely the same cgroup will perform
several allocations. Both the slab allocator and the page allocator
have per-cpu caches for the same reason. I don't really understand
what the argument is.

> > +
> > +	refill_obj_stock(objcg, PAGE_SIZE - size);
> 
> And the third problem is here. Percpu allocations (on which accounting I'm
> working right now) can be larger than a page.

How about this?

	nr_pages = round_up(size, PAGE_SIZE);
	try_charge(objcg->memcg, nr_pages);
	refill_obj_stock(objcg, size % PAGE_SIZE);

> This is fairly small issue in comparison to the first one. But it illustrates
> well the main point: we can't simple get a page from the existing API and
> sublease it in parts. The problem is that we need to break the main principle
> that a page belongs to a single memcg.

We can change the underlying assumptions of the existing API if they
are no longer correct. We don't have to invent a parallel stack.