The kmem_cache_alloc() is typically used to free memory allocated through 
the kernel memory cache (slab allocator). 

Using kmem_cache_free() for deallocation may be more reasonable.

Signed-off-by: Yan Zhen <yanzhen@vivo.com>
---
 mm/slab_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index da1b00665..0463df45e 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -491,7 +491,7 @@ kmem_buckets *kmem_buckets_create(const char *name, slab_flags_t flags,
 fail:
 	for (idx = 0; idx < ARRAY_SIZE(kmalloc_caches[KMALLOC_NORMAL]); idx++)
 		kmem_cache_destroy((*b)[idx]);
-	kfree(b);
+	kmem_cache_free(kmem_buckets_cache, b);
 
 	return NULL;
 }
-- 
2.34.1

> The kmem_cache_alloc() is typically used to free memory allocated through
> the kernel memory cache (slab allocator).

I find this wording confusing.


> Using kmem_cache_free() for deallocation may be more reasonable.

Will “imperative mood” become more desirable here?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.11-rc5#n94

Regards,
Markus

On Thu, 22 Aug 2024, Yan Zhen wrote:

> The kmem_cache_alloc() is typically used to free memory allocated through
> the kernel memory cache (slab allocator).

Well yes but since we removed SLOB we can use kfree() on any slab object.

> Using kmem_cache_free() for deallocation may be more reasonable.


It is more symmetric and looks better.

Reviewed-by: Christoph Lameter <cl@linux.com>

On 8/22/24 18:59, Christoph Lameter (Ampere) wrote:
> On Thu, 22 Aug 2024, Yan Zhen wrote:
> 
>> The kmem_cache_alloc() is typically used to free memory allocated through
>> the kernel memory cache (slab allocator).
> 
> Well yes but since we removed SLOB we can use kfree() on any slab object.
> 
>> Using kmem_cache_free() for deallocation may be more reasonable.
> 
> 
> It is more symmetric and looks better.

Right, added to slab/for-next, thanks.

> Reviewed-by: Christoph Lameter <cl@linux.com>

On 8/27/24 16:49, Vlastimil Babka wrote:
> On 8/22/24 18:59, Christoph Lameter (Ampere) wrote:
>> On Thu, 22 Aug 2024, Yan Zhen wrote:
>> 
>>> The kmem_cache_alloc() is typically used to free memory allocated through
>>> the kernel memory cache (slab allocator).

oh but _alloc() is used to alloc, not free.

>> 
>> Well yes but since we removed SLOB we can use kfree() on any slab object.
>> 
>>> Using kmem_cache_free() for deallocation may be more reasonable.

so I've reworded the commit log a bit.

    mm, slab: use kmem_cache_free() to free from kmem_buckets_cache

    In kmem_buckets_create(), the kmem_buckets object is allocated by
    kmem_cache_alloc() from kmem_buckets_cache, but in the failure case,
    it's freed by kfree(), which is not wrong, but using kmem_cache_free()
    is the more common pattern, so use it.



>> It is more symmetric and looks better.
> 
> Right, added to slab/for-next, thanks.
> 
>> Reviewed-by: Christoph Lameter <cl@linux.com>
>