| Message ID | 20240822022704.1195439-1-yanzhen@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] mm:slab:use kmem_cache_free() to free | expand |
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> >
> 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
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; }
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(-)