On Fri, Jun 19, 2020 at 05:16:02PM -0700, Shakeel Butt wrote:
> On Mon, Jun 8, 2020 at 4:07 PM Roman Gushchin <guro@xxxxxx> wrote:
> >
> > Store the obj_cgroup pointer in the corresponding place of
> > page->obj_cgroups for each allocated non-root slab object.
> > Make sure that each allocated object holds a reference to obj_cgroup.
> >
> > Objcg pointer is obtained from the memcg->objcg dereferencing
> > in memcg_kmem_get_cache() and passed from pre_alloc_hook to
> > post_alloc_hook. Then in case of successful allocation(s) it's
> > getting stored in the page->obj_cgroups vector.
> >
> > The objcg obtaining part look a bit bulky now, but it will be simplified
> > by next commits in the series.
> >
> > Signed-off-by: Roman Gushchin <guro@xxxxxx>
> > Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
>
> One nit below otherwise:
>
> Reviewed-by: Shakeel Butt <shakeelb@xxxxxxxxxx>
>
> > ---
> [snip]
> > +static inline void memcg_slab_post_alloc_hook(struct kmem_cache *s,
> > + struct obj_cgroup *objcg,
> > + size_t size, void **p)
> > +{
> > + struct page *page;
> > + unsigned long off;
> > + size_t i;
> > +
> > + for (i = 0; i < size; i++) {
> > + if (likely(p[i])) {
> > + page = virt_to_head_page(p[i]);
> > + off = obj_to_index(s, page, p[i]);
> > + obj_cgroup_get(objcg);
> > + page_obj_cgroups(page)[off] = objcg;
> > + }
> > + }
> > + obj_cgroup_put(objcg);
>
> Nit: we get the objcg reference in memcg_kmem_get_cache(), doesn't it
> look cleaner to put that reference in memcg_kmem_put_cache() instead
> of here.
memcg_kmem_put_cache() will go away completely later in the series.
I know the code might look sub-optimal and messy on some stages,
but it's only because there is a big transition from the original
to the final state, and I don't wanna to increase intermediate diffs.
Please, take a look at the final result.
