On Fri, Jun 07, 2013 at 05:54:06PM +0200, Michal Hocko wrote: > On Fri 07-06-13 18:45:17, Glauber Costa wrote: > > On 06/07/2013 06:12 PM, Michal Hocko wrote: > > >On Fri 07-06-13 14:11:53, Glauber Costa wrote: > > >>On 06/07/2013 01:21 PM, Michal Hocko wrote: > > >>>On Tue 04-06-13 18:10:59, Glauber Costa wrote: > > >>>>The memory we used to hold the memcg arrays is currently accounted to > > >>>>the current memcg. > > >>> > > >>>Maybe I have missed a train but I thought that only some caches are > > >>>tracked and those have to be enabled explicitly by using __GFP_KMEMCG in > > >>>gfp flags. > > >> > > >>No, all caches are tracked. This was set a long time ago, and only a > > >>very few initial versions differed from this. This barely changed over > > >>the lifetime of the memcg patchset. > > >> > > >>You probably got confused, due to the fact that only some *allocations* > > > > > >OK, I was really imprecise. Of course any type of cache might be tracked > > >should the allocation (which takes gfp) say so. What I have missed is > > >that not only stack allocations say so but also kmalloc itself enforces > > >that rather than the actual caller of kmalloc. This is definitely new > > >to me. And it is quite confusing that the flag is set only for large > > >allocations (kmalloc_order) or am I just missing other parts where > > >__GFP_KMEMCG is set unconditionally? > > > > > >I really have to go and dive into the code. > > > > > > > Here is where you are getting your confusion: we don't track caches, > > we track *pages*. > > > > Everytime you pass GFP_KMEMCG to a *page* allocation, it gets tracked. > > Every memcg cache - IOW, a memcg copy of a slab cache, sets > > GFP_KMEMCG for all its allocations. > > yes that is clear to me. > > > Now, the slub - and this is really an implementation detail - > > doesn't have caches for high order kmalloc caches. Instead, it gets > > pages directly from the page allocator. So we have to mark them > > explicitly. (they are a cache, they are just not implemented as > > such) > > I am still confused. If kmalloc_large_node is called because the size of > the object is larger than SLUB_MAX_SIZE then __GFP_KMEMCG is added > automatically regardless what _caller_ of kmalloc said. What am I > missing? > You are not missing anything, I am. It was not a problem since now because all allocations being bypassed were pretty small - so I got blinded by this. The logic I have explained to you is correct and will for 100 % of the time for the SLAB. The SLUB allocator, however, will ignore our bypassing request because it will never get to memcg_kmem_get_cache. It doesn't hurt to have the bypass check at memcg_kmem_newpage_charge as well, so I will add it - Thank you very much for noticing this. The only situation in which it *could* hurt to have an extra check in there, is if we decide to bypass the allocations somewhere inside the slab caches themselves, in such a way that we would select a memcg cache at memcg_kmem_get_cache, but then insert a non-memcg page in it because between the cache selection and the allocation there was a bypass request. As long as we keep the bypass requests memcg-internal, it should not be a problem. So in a summary: We will need two patches instead of one to tackle this. I will send you shortly. -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
