On 2023/12/5 03:34, Vlastimil Babka wrote:
> Currently, when __kmem_cache_alloc_bulk() fails, it frees back the
> objects that were allocated before the failure, using
> kmem_cache_free_bulk(). Because kmem_cache_free_bulk() calls the free
> hooks (KASAN etc.) and those expect objects that were processed by the
> post alloc hooks, slab_post_alloc_hook() is called before
> kmem_cache_free_bulk().
>
> This is wasteful, although not a big concern in practice for the rare
> error path. But in order to efficiently handle percpu array batch refill
> and free in the near future, we will also need a variant of
> kmem_cache_free_bulk() that avoids the free hooks. So introduce it now
> and use it for the failure path.
>
> As a consequence, __kmem_cache_alloc_bulk() no longer needs the objcg
> parameter, remove it.
The objects may have been charged before, but it seems __kmem_cache_alloc_bulk()
forget to uncharge them? I can't find "uncharge" in do_slab_free(), or maybe
the bulk interface won't be used on chargeable slab?
Thanks.
>
> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
> ---
> mm/slub.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index d7b0ca6012e0..0742564c4538 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4478,6 +4478,27 @@ int build_detached_freelist(struct kmem_cache *s, size_t size,
> return same;
> }
>
> +/*
> + * Internal bulk free of objects that were not initialised by the post alloc
> + * hooks and thus should not be processed by the free hooks
> + */
> +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p)
> +{
> + if (!size)
> + return;
> +
> + do {
> + struct detached_freelist df;
> +
> + size = build_detached_freelist(s, size, p, &df);
> + if (!df.slab)
> + continue;
> +
> + do_slab_free(df.s, df.slab, df.freelist, df.tail, df.cnt,
> + _RET_IP_);
> + } while (likely(size));
> +}
> +
> /* Note that interrupts must be enabled when calling this function. */
> void kmem_cache_free_bulk(struct kmem_cache *s, size_t size, void **p)
> {
> @@ -4499,7 +4520,7 @@ EXPORT_SYMBOL(kmem_cache_free_bulk);
>
> #ifndef CONFIG_SLUB_TINY
> static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
> - size_t size, void **p, struct obj_cgroup *objcg)
> + size_t size, void **p)
> {
> struct kmem_cache_cpu *c;
> unsigned long irqflags;
> @@ -4563,14 +4584,13 @@ static inline int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
>
> error:
> slub_put_cpu_ptr(s->cpu_slab);
> - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size);
> - kmem_cache_free_bulk(s, i, p);
> + __kmem_cache_free_bulk(s, i, p);
> return 0;
>
> }
> #else /* CONFIG_SLUB_TINY */
> static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
> - size_t size, void **p, struct obj_cgroup *objcg)
> + size_t size, void **p)
> {
> int i;
>
> @@ -4593,8 +4613,7 @@ static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags,
> return i;
>
> error:
> - slab_post_alloc_hook(s, objcg, flags, i, p, false, s->object_size);
> - kmem_cache_free_bulk(s, i, p);
> + __kmem_cache_free_bulk(s, i, p);
> return 0;
> }
> #endif /* CONFIG_SLUB_TINY */
> @@ -4614,7 +4633,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
> if (unlikely(!s))
> return 0;
>
> - i = __kmem_cache_alloc_bulk(s, flags, size, p, objcg);
> + i = __kmem_cache_alloc_bulk(s, flags, size, p);
>
> /*
> * memcg and kmem_cache debug support and memory initialization.
>
