linux-kernel - Re: [PATCH -mm 2/4] sl[au]b: charge slabs to memcg explicitly

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]

Message-ID: <20140327203829.GA28590@dhcp22.suse.cz>
Date:	Thu, 27 Mar 2014 13:38:29 -0700
From:	Michal Hocko <mhocko@...e.cz>
To:	Vladimir Davydov <vdavydov@...allels.com>
Cc:	akpm@...ux-foundation.org, hannes@...xchg.org, glommer@...il.com,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, devel@...nvz.org,
	Christoph Lameter <cl@...ux-foundation.org>,
	Pekka Enberg <penberg@...nel.org>
Subject: Re: [PATCH -mm 2/4] sl[au]b: charge slabs to memcg explicitly

On Thu 27-03-14 11:38:30, Vladimir Davydov wrote:
> On 03/27/2014 01:58 AM, Michal Hocko wrote:
> > On Wed 26-03-14 19:28:05, Vladimir Davydov wrote:
> >> We have only a few places where we actually want to charge kmem so
> >> instead of intruding into the general page allocation path with
> >> __GFP_KMEMCG it's better to explictly charge kmem there. All kmem
> >> charges will be easier to follow that way.
> >>
> >> This is a step towards removing __GFP_KMEMCG. It removes __GFP_KMEMCG
> >> from memcg caches' allocflags. Instead it makes slab allocation path
> >> call memcg_charge_kmem directly getting memcg to charge from the cache's
> >> memcg params.
> > Yes, removing __GFP_KMEMCG is definitely a good step. I am currently at
> > a conference and do not have much time to review this properly (even
> > worse will be on vacation for the next 2 weeks) but where did all the
> > static_key optimization go? What am I missing.
> 
> I expected this question, because I want somebody to confirm if we
> really need such kind of optimization in the slab allocation path. From
> my POV, since we thrash cpu caches there anyway by calling alloc_pages,
> wrapping memcg_charge_slab in a static branch wouldn't result in any
> noticeable performance boost.
> 
> I do admit we benefit from static branching in memcg_kmem_get_cache,
> because this one is called on every kmem object allocation, but slab
> allocations happen much rarer.
> 
> I don't insist on that though, so if you say "no", I'll just add
> __memcg_charge_slab and make memcg_charge_slab call it if the static key
> is on, but may be, we can avoid such code bloating?

I definitely do not insist on static branching at places where it
doesn't help much. The less tricky code we will have the better. Please
document this in the changelog and drop a comment in memcg_charge_slab
which would tell us why we do not have to check for kmem enabling.

Thanks!
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/