Re: [PATCH v7 17/19] mm: memcg/slab: use a single set of kmem_caches for all allocations
On Sat, Jul 18, 2020 at 10:24:49AM -0700, Guenter Roeck wrote:
> On Tue, Jun 23, 2020 at 10:40:35AM -0700, Roman Gushchin 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.
> >
> > Suggested-by: Johannes Weiner
> > Signed-off-by: Roman Gushchin
> > Reviewed-by: Vlastimil Babka
> > Reviewed-by: Shakeel Butt
>
> This patch results in:
>
> {standard input}: Assembler messages:
> {standard input}:140: Warning: macro instruction expanded into multiple
> instructions
> mm/slub.c: In function 'slab_alloc.constprop':
> mm/slub.c:2897:30: error: inlining failed in call to always_inline
> 'slab_alloc.constprop': recursive inlining
> static __always_inline void *slab_alloc(struct kmem_cache *s,
>
> and many similar messages when trying to build mips:64r6el_defconfig
> or mips:defconfig. Bisect log attached.
Hello, Guenter!
Thank you for reporting the problem!
Actually, I've already fixed it, and Andrew pulled the fix yesterday.
So in few days the fix should appear in the next tree.
Sorry for the inconvenience and please let me know if the problem will persist
after a couple of days.
Roman
Re: [PATCH v7 17/19] mm: memcg/slab: use a single set of kmem_caches for all allocations
On Tue, Jun 23, 2020 at 10:40:35AM -0700, Roman Gushchin 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.
>
> Suggested-by: Johannes Weiner
> Signed-off-by: Roman Gushchin
> Reviewed-by: Vlastimil Babka
> Reviewed-by: Shakeel Butt
This patch results in:
{standard input}: Assembler messages:
{standard input}:140: Warning: macro instruction expanded into multiple
instructions
mm/slub.c: In function 'slab_alloc.constprop':
mm/slub.c:2897:30: error: inlining failed in call to always_inline
'slab_alloc.constprop': recursive inlining
static __always_inline void *slab_alloc(struct kmem_cache *s,
and many similar messages when trying to build mips:64r6el_defconfig
or mips:defconfig. Bisect log attached.
Guenter
---
# bad: [aab7ee9f8ff0110bfcd594b33dc33748dc1baf46] Add linux-next specific files
for 20200717
# good: [11ba468877bb23f28956a35e896356252d63c983] Linux 5.8-rc5
git bisect start 'HEAD' 'v5.8-rc5'
# good: [4d55a7a1298d197755c1a0f4512f56917e938a83] Merge remote-tracking branch
'crypto/master'
git bisect good 4d55a7a1298d197755c1a0f4512f56917e938a83
# good: [e63bf5dcce255302e355cb2277a3a39c83752c92] Merge remote-tracking branch
'devicetree/for-next'
git bisect good e63bf5dcce255302e355cb2277a3a39c83752c92
# good: [94d932ec3afb923efd8c736974f8316413175a5b] Merge remote-tracking branch
'thunderbolt/next'
git bisect good 94d932ec3afb923efd8c736974f8316413175a5b
# good: [5ddd2e0dbe8fceb80b0b36bd38a32217be7a04a5] Merge remote-tracking branch
'livepatching/for-next'
git bisect good 5ddd2e0dbe8fceb80b0b36bd38a32217be7a04a5
# bad: [40346f79983caf46fb92f779b0353422d43580a9] ipc/shm.c: Remove the
superfluous break
git bisect bad 40346f79983caf46fb92f779b0353422d43580a9
# bad: [0b917599517f71ddef5f7274a8199a33cecd49b2] kasan: update required
compiler versions in documentation
git bisect bad 0b917599517f71ddef5f7274a8199a33cecd49b2
# good: [7822c5f77725d5bf4ea48f155b0aa3827db19423] tmpfs: per-superblock i_ino
support
git bisect good 7822c5f77725d5bf4ea48f155b0aa3827db19423
# bad: [c5b15b89803e3ed2810be285def5f4836e5ee629] mm, memcg: reclaim more
aggressively before high allocator throttling
git bisect bad c5b15b89803e3ed2810be285def5f4836e5ee629
# good: [2b6d98a0b0cb5ff828228c6a094813c4919727da] mm: memcg/slab: remove
redundant check in memcg_accumulate_slabinfo()
git bisect good 2b6d98a0b0cb5ff828228c6a094813c4919727da
# bad: [d32b702628530c68b4147d410b4cdf21610e9f93] mm: memcg/percpu: per-memcg
percpu memory statistics
git bisect bad d32b702628530c68b4147d410b4cdf21610e9f93
# bad: [b109396be9be1b8fd91fa4c70bd73a0e93722274] percpu: return number of
released bytes from pcpu_free_area()
git bisect bad b109396be9be1b8fd91fa4c70bd73a0e93722274
# bad: [6cee58aca5d334ee8195a711e4eb61a05e5f7eb5] kselftests: cgroup: add
kernel memory accounting tests
git bisect bad 6cee58aca5d334ee8195a711e4eb61a05e5f7eb5
# bad: [2528f5d4f3c139035dc3adcbfb6c63ca14c840f0] mm: memcg/slab: use a single
set of kmem_caches for all allocations
git bisect bad 2528f5d4f3c139035dc3adcbfb6c63ca14c840f0
# first bad commit: [2528f5d4f3c139035dc3adcbfb6c63ca14c840f0] mm: memcg/slab:
use a single set of kmem_caches for all allocations
Re: [PATCH v7 17/19] mm: memcg/slab: use a single set of kmem_caches for all allocations
On Mon, Jun 22, 2020 at 6:58 PM Roman Gushchin 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. > > Suggested-by: Johannes Weiner > Signed-off-by: Roman Gushchin > Reviewed-by: Vlastimil Babka Reviewed-by: Shakeel Butt
