Re: [PATCH v2 4/6] slab: add SLAB_ACCOUNT flag

Vladimir Davydov
2015-11-14T11:29:47Z

On Thu, Nov 12, 2015 at 05:17:41PM +0100, Michal Hocko wrote:
> On Tue 10-11-15 21:34:05, Vladimir Davydov wrote:
> > Currently, if we want to account all objects of a particular kmem cache,
> > we have to pass __GFP_ACCOUNT to each kmem_cache_alloc call, which is
> > inconvenient. This patch introduces SLAB_ACCOUNT flag which if passed to
> > kmem_cache_create will force accounting for every allocation from this
> > cache even if __GFP_ACCOUNT is not passed.
>
> Yes this is much better and less error prone for dedicated caches.
>
> > This patch does not make any of the existing caches use this flag - it
> > will be done later in the series.
> >
> > Note, a cache with SLAB_ACCOUNT cannot be merged with a cache w/o
> > SLAB_ACCOUNT, i.e. using this flag will probably reduce the number of
> > merged slabs even if kmem accounting is not used (only compiled in).
>
> I would expect some reasoning why this is the case. Why cannot caches of
> the same memcg be merged? I remember you have mentioned something in the
> previous discussion with Tejun but it should be in the changelog as well
> IMO.

Here goes an extended version of the last paragraph, hope it makes
everything clear:

"""
Note, a cache with SLAB_ACCOUNT cannot be merged with a cache w/o
SLAB_ACCOUNT, because merged caches share the same kmem_cache struct and
hence cannot have different sets of SLAB_* flags. Thus using this flag
will probably reduce the number of merged slabs even if kmem accounting
is not used (only compiled in).
"""

Andrew, could you please update the commit message?

>
> > Suggested-by: Tejun Heo <tj@kernel.org>
> > Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
>
> I am not sufficiently qualified to judge the slab implementation
> specifics but for the overal approach
>
> Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!