Hi Kuan-Wei,
On Sun, Oct 20, 2024 at 6:02 AM Kuan-Wei Chiu visitorckw@gmail.com wrote:
All current min heap API functions are marked with '__always_inline'. However, as the number of users increases, inlining these functions everywhere leads to a increase in kernel size.
In performance-critical paths, such as when perf events are enabled and min heap functions are called on every context switch, it is important to retain the inline versions for optimal performance. To balance this, the original inline functions are kept, and additional non-inline versions of the functions have been added in lib/min_heap.c.
Link: https://lore.kernel.org/20240522161048.8d8bbc7b153b4ecd92c50666@linux-founda... Suggested-by: Andrew Morton akpm@linux-foundation.org Signed-off-by: Kuan-Wei Chiu visitorckw@gmail.com
Thanks for your patch, which is now commit 92a8b224b833e82d ("lib/min_heap: introduce non-inline versions of min heap API functions") upstream.
--- a/include/linux/min_heap.h +++ b/include/linux/min_heap.h
@@ -50,33 +50,33 @@ void __min_heap_init(min_heap_char *heap, void *data, int size) heap->data = heap->preallocated; }
-#define min_heap_init(_heap, _data, _size) \
__min_heap_init((min_heap_char *)_heap, _data, _size)+#define min_heap_init_inline(_heap, _data, _size) \
__min_heap_init_inline((min_heap_char *)_heap, _data, _size)
Casting macro parameters without any further checks prevents the compiler from detecting silly mistakes. Would it be possible to add safety-nets here and below, using e.g. container_of() or typeof() checks?
--- a/lib/Kconfig +++ b/lib/Kconfig @@ -777,3 +777,6 @@ config POLYNOMIAL
config FIRMWARE_TABLE bool
+config MIN_HEAP
bool
Perhaps tristate? See also below.
--- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2279,6 +2279,7 @@ config TEST_LIST_SORT config TEST_MIN_HEAP tristate "Min heap test" depends on DEBUG_KERNEL || m
select MIN_HEAP
Ideally, tests should not select functionality, to prevent increasing the attack vector by merely enabling (modular) tests.
In this particular case, just using "depends on MIN_HEAP" is not an option, as MIN_HEAP is not user-visible, and thus cannot be enabled by the user on its own. However, making MIN_HEAP tristate could be a first step for the modular case.
The builtin case is harder to fix, as e.g.
depends on MIN_HEAP || COMPILE_TEST select MIN_HEAP if COMPILE_TEST
would still trigger a recursive dependency error.
Alternatively, the test could just keep on using the inline variants, unless CONFIG_MIN_HEAP=y? Or event test both for the latter?
help Enable this to turn on min heap function tests. This test is executed only once during system boot (so affects only boot time),
Gr{oetje,eeting}s,
Geert