| Message ID | 20220221105336.522086-4-42.hyeyoo@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | slab cleanups | expand |
On Mon, Feb 21, 2022 at 10:53:34AM +0000, Hyeonggon Yoo wrote: > SLAB's kfree() does not support freeing an object that is allocated from > kmalloc_large(). Fix this as SLAB do not pass requests larger than > KMALLOC_MAX_CACHE_SIZE directly to page allocator. I was wondering if we wanted to go in the other direction and get rid of kmalloc cache sizes larger than, say, 64kB from the SLAB allocator.
On Mon, Feb 21, 2022 at 03:53:39PM +0000, Matthew Wilcox wrote: > On Mon, Feb 21, 2022 at 10:53:34AM +0000, Hyeonggon Yoo wrote: > > SLAB's kfree() does not support freeing an object that is allocated from > > kmalloc_large(). Fix this as SLAB do not pass requests larger than > > KMALLOC_MAX_CACHE_SIZE directly to page allocator. > > I was wondering if we wanted to go in the other direction and get rid of > kmalloc cache sizes larger than, say, 64kB from the SLAB allocator. Good point. Hmm.. I don't think SLAB is benefiting from queueing that large objects, and maximum size is still limited to what buddy allocator supports. I'll try reducing kmalloc caches up to order-1 page like SLUB. That would be easier to maintain. Thanks, Hyeonggon
On Tue, Feb 22, 2022 at 08:10:32AM +0000, Hyeonggon Yoo wrote: > On Mon, Feb 21, 2022 at 03:53:39PM +0000, Matthew Wilcox wrote: > > On Mon, Feb 21, 2022 at 10:53:34AM +0000, Hyeonggon Yoo wrote: > > > SLAB's kfree() does not support freeing an object that is allocated from > > > kmalloc_large(). Fix this as SLAB do not pass requests larger than > > > KMALLOC_MAX_CACHE_SIZE directly to page allocator. > > > > I was wondering if we wanted to go in the other direction and get rid of > > kmalloc cache sizes larger than, say, 64kB from the SLAB allocator. > > Good point. > > Hmm.. I don't think SLAB is benefiting from queueing that large objects, > and maximum size is still limited to what buddy allocator supports. > > I'll try reducing kmalloc caches up to order-1 page like SLUB. > That would be easier to maintain. If you have time to investigate these kinds of things, I think SLUB would benefit from caching order-2 and order-3 slabs as well. Maybe not so much now that Mel included order-2 and order-3 caching in the page allocator. But it'd be interesting to have numbers.
On Tue, Feb 22, 2022 at 07:59:08PM +0000, Matthew Wilcox wrote: > On Tue, Feb 22, 2022 at 08:10:32AM +0000, Hyeonggon Yoo wrote: > > On Mon, Feb 21, 2022 at 03:53:39PM +0000, Matthew Wilcox wrote: > > > On Mon, Feb 21, 2022 at 10:53:34AM +0000, Hyeonggon Yoo wrote: > > > > SLAB's kfree() does not support freeing an object that is allocated from > > > > kmalloc_large(). Fix this as SLAB do not pass requests larger than > > > > KMALLOC_MAX_CACHE_SIZE directly to page allocator. > > > > > > I was wondering if we wanted to go in the other direction and get rid of > > > kmalloc cache sizes larger than, say, 64kB from the SLAB allocator. > > > > Good point. > > > > Hmm.. I don't think SLAB is benefiting from queueing that large objects, > > and maximum size is still limited to what buddy allocator supports. > > > > I'll try reducing kmalloc caches up to order-1 page like SLUB. > > That would be easier to maintain. > > If you have time to investigate these kinds of things, I think SLUB would > benefit from caching order-2 and order-3 slabs as well. Maybe not so much > now that Mel included order-2 and order-3 caching in the page allocator. > But it'd be interesting to have numbers. That's interesting topic. But I think this is slightly different topic. AFAIK It's rare that a workload would benefit more from using slab for large size objects (8K, 16K, ... etc) than using page allocator. And yeah, caching high order slabs may affect the numbers even if page allocator caches high order pages. SLUB already caches them and SLUB can cache more slabs by tuning number of cpu partial slabs (s->cpu_partial_slabs) and number of node partial slabs. (s->min_partial) I need to investigate what actually Mel did and learn how it affects SLUB. So it will take some time. Thanks! -- Hyeonggon
On 2/21/22 11:53, Hyeonggon Yoo wrote: > SLAB's kfree() does not support freeing an object that is allocated from > kmalloc_large(). Fix this as SLAB do not pass requests larger than > KMALLOC_MAX_CACHE_SIZE directly to page allocator. AFAICS this issue is limited to build-time constant sizes. Might be better to make this a build error rather than build-time NULL? > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
On Thu, Feb 24, 2022 at 01:48:47PM +0100, Vlastimil Babka wrote: > On 2/21/22 11:53, Hyeonggon Yoo wrote: > > SLAB's kfree() does not support freeing an object that is allocated from > > kmalloc_large(). Fix this as SLAB do not pass requests larger than > > KMALLOC_MAX_CACHE_SIZE directly to page allocator. > > AFAICS this issue is limited to build-time constant sizes. Might be better > to make this a build error rather than build-time NULL? Right. And sounds better. But what about another direction as Matthew said: passing large requests to page allocator like SLUB? I think it's better for maintenance. Any obstacles for this direction? Thank you! > > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
On 2/24/22 14:31, Hyeonggon Yoo wrote: > On Thu, Feb 24, 2022 at 01:48:47PM +0100, Vlastimil Babka wrote: >> On 2/21/22 11:53, Hyeonggon Yoo wrote: >> > SLAB's kfree() does not support freeing an object that is allocated from >> > kmalloc_large(). Fix this as SLAB do not pass requests larger than >> > KMALLOC_MAX_CACHE_SIZE directly to page allocator. >> >> AFAICS this issue is limited to build-time constant sizes. Might be better >> to make this a build error rather than build-time NULL? > > Right. And sounds better. But what about another direction as Matthew said: > passing large requests to page allocator like SLUB? Sounds like a good idea, that would reduce the number of kmalloc caches with SLAB, and I expect also simplify the common code further. > I think it's better for maintenance. Any obstacles for this direction? > > Thank you! > >> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> >
diff --git a/include/linux/slab.h b/include/linux/slab.h index 37bde99b74af..aeda3e863f2b 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -564,15 +564,19 @@ static __always_inline __alloc_size(1) void *kmalloc_large(size_t size, gfp_t fl * Try really hard to succeed the allocation but fail * eventually. */ +#ifndef CONFIG_SLOB static __always_inline __alloc_size(1) void *kmalloc(size_t size, gfp_t flags) { if (__builtin_constant_p(size)) { -#ifndef CONFIG_SLOB unsigned int index; -#endif - if (size > KMALLOC_MAX_CACHE_SIZE) - return kmalloc_large(size, flags); -#ifndef CONFIG_SLOB + + if (size > KMALLOC_MAX_CACHE_SIZE) { + if (IS_ENABLED(CONFIG_SLUB)) + return kmalloc_large(size, flags); + else + return NULL; + } + index = kmalloc_index(size); if (!index) @@ -581,10 +585,17 @@ static __always_inline __alloc_size(1) void *kmalloc(size_t size, gfp_t flags) return kmem_cache_alloc_trace( kmalloc_caches[kmalloc_type(flags)][index], flags, size); -#endif } return __kmalloc(size, flags); } +#else +static __always_inline __alloc_size(1) void *kmalloc(size_t size, gfp_t flags) +{ + if (__builtin_constant_p(size) && size > KMALLOC_MAX_CACHE_SIZE) + return kmalloc_large(size, flags); + return __kmalloc(size, flags); +} +#endif static __always_inline __alloc_size(1) void *kmalloc_node(size_t size, gfp_t flags, int node) {
SLAB's kfree() does not support freeing an object that is allocated from kmalloc_large(). Fix this as SLAB do not pass requests larger than KMALLOC_MAX_CACHE_SIZE directly to page allocator. Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> --- include/linux/slab.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)