On Thu, Jun 18, 2020 at 09:33:08AM +0200, Vlastimil Babka wrote: > On 6/18/20 2:35 AM, Roman Gushchin wrote: > > On Wed, Jun 17, 2020 at 04:35:28PM -0700, Andrew Morton wrote: > >> On Mon, 8 Jun 2020 16:06:52 -0700 Roman Gushchin <guro@xxxxxx> wrote: > >> > >> > Instead of having two sets of kmem_caches: one for system-wide and > >> > non-accounted allocations and the second one shared by all accounted > >> > allocations, we can use just one. > >> > > >> > The idea is simple: space for obj_cgroup metadata can be allocated > >> > on demand and filled only for accounted allocations. > >> > > >> > It allows to remove a bunch of code which is required to handle > >> > kmem_cache clones for accounted allocations. There is no more need > >> > to create them, accumulate statistics, propagate attributes, etc. > >> > It's a quite significant simplification. > >> > > >> > Also, because the total number of slab_caches is reduced almost twice > >> > (not all kmem_caches have a memcg clone), some additional memory > >> > savings are expected. On my devvm it additionally saves about 3.5% > >> > of slab memory. > >> > > >> > >> This ran afoul of Vlastimil's "mm, slab/slub: move and improve > >> cache_from_obj()" > >> (http://lkml.kernel.org/r/20200610163135.17364-10-vbabka@xxxxxxx). I > >> resolved things as below. Not too sure about slab.c's > >> cache_from_obj()... > > > > It can actually be as simple as: > > static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x) > > { > > return s; > > } > > > > But I wonder if we need it at all, or maybe we wanna rename it to > > something like obj_check_kmem_cache(void *obj, struct kmem_cache *s), > > because it has now only debug purposes. > > > > Let me and Vlastimil figure it out and send a follow-up patch. > > Your version is definitely correct. > > Well, Kees wants to restore the common version of cache_from_obj() [1] for SLAB > hardening. > > To prevent all that back and forth churn entering git history, I think the best > is for me to send a -fix to my patch that is functionally same while keeping the > common function, and then this your patch should only have a minor conflict and > Kees can rebase his patches on top to become much smaller? Sounds good to me! Thanks!
