Make use of kvmalloc if available

[c0d3z3r0]

Motivation and Context

This is a spin-off of #8384

Modifications:

• removed kvmalloc interface
• moved kvmalloc stuff into vmem (Well, actually it was there before, but implemented with its own KM_* flag)
• remove new but currently unused KM_ONCE flag implementation
• removed spl_kvfree in favor of spl_kmem_free_impl
• code style fixes
• adapt commit message
• documentation

This PR introduces several improvements / fixes:

This patch implements use of kvmalloc for GFP_KERNEL allocations, which
may increase performance if the allocator is able to allocate physical
memory, if kvmalloc is available as a public kernel interface (since
v4.12). Otherwise it will simply fall back to virtual memory (vmalloc).

Also fix vmem_alloc implementation which can lead to slow allocations
since the first attempt with kmalloc does not make use of the noretry
flag but tells the linux kernel to retry several times before it fails.

This adds a new KMC_KVMEM flag was added to enforce use of the
kvmalloc allocator in kmem_cache_create even for large blocks, which
may also increase performance in some specific cases (e.g. zstd), too.

Make use of KMC_KVMEM for spl_zlib_workspace_cache, ddt_cache, zio_buf
and zio_data_buf.

Add missing documentation for some KMC flags

Make use of kvmalloc if available and fix vmem_alloc implementation

Signed-off-by: Sebastian Gottschall s.gottschall@dd-wrt.com
Signed-off-by: Michael Niewöhner foss@mniewoehner.de

Description

How Has This Been Tested?

Running stable on Debian testing, currently kernel 5.2.0, since months with very low load. @BrainSlayer did stress tests (see #8384).

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.

[c0d3z3r0]

Completely reworked and rebased onto master.

I also temporarily removed the vmem allocation retry fix. I'll readd this again as a separate commit later.

[behlendorf]

Completely reworked and rebased onto master.

Thanks! I'll take a proper look at this tomorrow, but I happened to notice the updated code in now triggering the allocation size warning for several slabs.

[BrainSlayer]

Am 17.07.2019 um 21:05 schrieb Michael Niewöhner:

***@***.**** commented on this pull request. ------------------------------------------------------------------------ In module/spl/spl-kmem-cache.c <#9034 (comment)>: > } else { - ptr = __vmalloc(size, lflags | __GFP_HIGHMEM, PAGE_KERNEL); + /* + * GFP_KERNEL allocations can safe use kvmalloc allocator which + * may increase performance since vmalloc overhead might not + * be applicated since enough contiguous memory was available + * at allocation time. unfortunatly the slab statistic will + * show allocations as VMEM without any care if kmalloc was + * used in reality. but this is just cosmetic + */ Hmmm.... don't we need a check for size <= spl_kmem_alloc_max in |kv_alloc|, too?

no. kv_alloc must always succeed. no matter which size is allocated. i want physical mem at any size if available. this is how its used in zstd

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9034>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2WNEY3DPOPX4WWTNJE6D3P75UQVANCNFSM4IC7IAVA>.

[BrainSlayer]

Am 17.07.2019 um 22:01 schrieb Michael Niewöhner:

Completely reworked and rebased onto master. I also temporarily remove the vmem allocation retry fix. I'll readd this again as a separate commit later.

you removed also the whole kvmalloc implementation for kernels without this function. so the current code is broken you just kall kvmalloc if its available. so this code is now unusable for me in the zstd code since it does not allow me to allocate bigger physical memory anymore if available.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9034>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2WNEZICXIGDMX2PBLRAETP753BZANCNFSM4IC7IAVA>.

[BrainSlayer]

Am 17.07.2019 um 22:01 schrieb Michael Niewöhner:

Completely reworked and rebased onto master. I also temporarily remove the vmem allocation retry fix. I'll readd this again as a separate commit later.

and your code introduces also bad bugs any call of spl_kmem_alloc_impl with GFP_ATOMIC will return NULL now see         if ((size > spl_kmem_alloc_max) || use_vmem ||             !((flags & GFP_KERNEL) != GFP_KERNEL)) { Am 17.07.2019 um 22:01 schrieb Michael Niewöhner:

…

Completely reworked and rebased onto master. I also temporarily remove the vmem allocation retry fix. I'll readd this again as a separate commit later. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9034>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2WNEZICXIGDMX2PBLRAETP753BZANCNFSM4IC7IAVA>.

[c0d3z3r0]

@BrainSlayer WIP ;)

[c0d3z3r0]

Hmmm.... don't we need a check for size <= spl_kmem_alloc_max in |kv_alloc|, too?

no. kv_alloc must always succeed. no matter which size is allocated. i want physical mem at any size if available. this is how its used in zstd

Ok... I will redo this later

[BrainSlayer]

Am 18.07.2019 um 09:54 schrieb Michael Niewöhner:

Hmmm.... don't we need a check for size <= spl_kmem_alloc_max in |kv_alloc|, too? no. kv_alloc must always succeed. no matter which size is allocated. i want physical mem at any size if available. this is how its used in zstd Ok... I will redo this later

thats hard without having a indepenend spl_kvmalloc implementation. the original zfs kmem_alloc code restricts the maximum allocatable physical memory per allocation. changing this behaviour is surelly not accepted by the maintainers. this is why i added the new api. so i can conditionally use it in the way i want it only for the codepieces i wrote or where it makes sense in some way. its also possible to use it unconditionally for the vmem allocation then. but not for kmem since it has a different behaviour unlike the original zfs allocation code

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9034>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2WNEYB2S4HMJV4KANU3ULQAAOTFANCNFSM4IC7IAVA>.

[c0d3z3r0]

want it only for the codepieces i wrote or where it makes sense in some
way.

no, we want to use it for any vmem alloc, as discussed above...

its also possible to use it unconditionally for the vmem allocation
then.

yes...

but not for kmem since it has a different behaviour unlike the
original zfs allocation code

again... I'll redo this later...

[c0d3z3r0]

@BrainSlayer can you have a look at this again, now?

[BrainSlayer]

this looks much better and allows me to override the kmem_alloc_max condition by directly calling spl_kvmalloc from codepaths where its needed like zstd. but what if i want to make use of kmem_cache_create using exclusive kvmem allocator without kmem_alloc_max condition. this was also a major part of the change. without my patch kmem_cache_create more or less always fails in zstd code due the nature of its allocation implementation. basicly kmem_cache_create does allocate using nosleep like gfp_atomic but this makes no sense if its just called once in a init module function but only from time critical contexts zstd does make use of very big kmem caches. if it fails it falls back to a custom allocator. but kmem_cache_create doesnt work for large blocks (alloc_max condition for vmem + nosleep problem using kmem) without my changes which make use of the extra kmc_kvmem flag so your patch will still work with the zstd code, but will decrease the performance since kmem_cache_create is unable to allocate the larger blocks. kmc_kvmem was using spl_kvmalloc exclusivly which has no alloc_max problem for vmem and is able to allocate kmem at any size without getting a kernel warning. while researching the freebsd variant of the zstd implementation this was a major point since freebsd allows such big kmem allocations Am 19.07.2019 um 09:45 schrieb Michael Niewöhner:

…

@BrainSlayer <https://github.com/BrainSlayer> can you have a look at this again, now? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9034>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2WNE2W6WM3PWIDUYHNN7DQAFWJ7ANCNFSM4IC7IAVA>.

[c0d3z3r0]

but kmem_cache_create doesnt work for large blocks
(alloc_max condition for vmem + nosleep problem using kmem) without my
changes which make use of the extra kmc_kvmem flag so your patch will
still work with the zstd code, but will decrease the performance since
kmem_cache_create is unable to allocate the larger blocks.

Ok, then I will readd KMC_KVMEM to make this possible again. Thanks for your explanations!

[c0d3z3r0]

but kmem_cache_create doesnt work for large blocks
(alloc_max condition for vmem + nosleep problem using kmem) without my
changes which make use of the extra kmc_kvmem flag so your patch will
still work with the zstd code, but will decrease the performance since
kmem_cache_create is unable to allocate the larger blocks.

Ok, then I will readd KMC_KVMEM to make this possible again. Thanks for your explanations!

@BrainSlayer done (but I am not sure about that spl proc stuff)

[behlendorf]

@c0d3z3r0 @BrainSlayer merged! Thank's for all your hard work on this improvement.

[c0d3z3r0]

@c0d3z3r0 @BrainSlayer merged! Thank's for all your hard work on this improvement.

yay! Thank you, too!

[BrainSlayer]

yeah now i have the painfull work to resolve the conflicts :-)