From: Caleb Kan page_owner keeps stable struct stack_record pointers in its stack list. It uses each record's count with a one-count bias to track live base pages and reads the stored entries directly. The trie backend provides neither a flat record layout nor an independent count field. Add STACK_DEPOT_FLAG_COUNTABLE to keep these records on the hash backend and deduplicate them separately from all non-countable records. Store the discriminator in a new u16 flags field and narrow size to u16. This keeps the record header size unchanged while covering the configured maximum of 256 frames. Do not otherwise split hash-table deduplication: ordinary and GET saves can continue to share records. Make COUNTABLE mutually exclusive with GET because the two flags assign incompatible meanings to the record count. Reject countable records in stack_depot_put() and require COUNTABLE in __stack_depot_get_stack_record(). Mark both page_owner save sites countable so its existing accounting and reporting continue to use stable hash records. Extend the KUnit coverage to verify direct-record access and isolation between countable and non-countable records. Signed-off-by: Caleb Kan --- include/linux/stackdepot.h | 14 ++++++++--- lib/stackdepot.c | 30 +++++++++++++++++++----- lib/tests/stackdepot_kunit.c | 55 ++++++++++++++++++++++++++++++++++++++++++++ mm/page_owner.c | 6 +++-- 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h index 96544fc684a5..788737eb0c4a 100644 --- a/include/linux/stackdepot.h +++ b/include/linux/stackdepot.h @@ -53,7 +53,8 @@ union handle_parts { struct stack_record { struct list_head hash_list; /* Links in the hash table */ u32 hash; /* Hash in hash table */ - u32 size; /* Number of stored frames */ + u16 size; /* Number of stored frames */ + u16 flags; union handle_parts handle; /* Constant after initialization */ refcount_t count; union { @@ -84,8 +85,9 @@ typedef u32 depot_flags_t; */ #define STACK_DEPOT_FLAG_CAN_ALLOC ((depot_flags_t)0x0001) #define STACK_DEPOT_FLAG_GET ((depot_flags_t)0x0002) +#define STACK_DEPOT_FLAG_COUNTABLE ((depot_flags_t)0x0004) -#define STACK_DEPOT_FLAGS_NUM 2 +#define STACK_DEPOT_FLAGS_NUM 3 #define STACK_DEPOT_FLAGS_MASK ((depot_flags_t)((1 << STACK_DEPOT_FLAGS_NUM) - 1)) /* @@ -144,6 +146,11 @@ static inline int stack_depot_early_init(void) { return 0; } * Users of this flag must also call stack_depot_put() when keeping the stack * trace is no longer required to avoid overflowing the refcount. * + * If STACK_DEPOT_FLAG_COUNTABLE is set in @depot_flags, stack depot stores the + * stack in hash-backed storage for callers that need direct stack_record count + * access. This flag does not imply %STACK_DEPOT_FLAG_CAN_ALLOC and is mutually + * exclusive with %STACK_DEPOT_FLAG_GET. + * * When trie storage is enabled, persistent non-refcounted saves use trie * storage. Constrained callers only look up existing stacks; they do not insert * a missing stack. Trie failures do not fall back to hash storage. @@ -190,7 +197,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, * * @handle: Stack depot handle * - * This function is only for internal purposes. + * This function is only for internal purposes. @handle must have been saved + * with %STACK_DEPOT_FLAG_COUNTABLE. * * Return: Returns a pointer to a stack_record struct */ diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 0278b7a013f1..1e5b9fc44618 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -2,10 +2,11 @@ /* * Stack depot - a stack trace storage that avoids duplication. * - * Internally, stack depot has two storage backends. Refcounted entries use the - * legacy hash table with contiguous stack records in stack pools. Persistent - * non-refcounted entries can use trie storage when enabled; trie nodes share - * common frame prefixes and are published through RCU children containers. + * Internally, stack depot has two storage backends. Refcounted entries and + * callers that request STACK_DEPOT_FLAG_COUNTABLE use the legacy hash table with + * contiguous stack records in stack pools. Persistent non-refcounted entries + * can use trie storage when enabled; trie nodes share common frame prefixes and + * are published through RCU children containers. * * Author: Alexander Potapenko * Copyright (C) 2016 Google, Inc. @@ -1022,6 +1023,7 @@ depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, dep /* Save the stack trace. */ stack->hash = hash; stack->size = nr_entries; + stack->flags = flags & STACK_DEPOT_FLAG_COUNTABLE; /* stack->handle is already filled in by depot_pop_free_pool(). */ memcpy(stack->entries, entries, flex_array_size(stack, entries, nr_entries)); @@ -1164,6 +1166,9 @@ static inline struct stack_record *find_stack(struct list_head *bucket, list_for_each_entry_rcu(stack, bucket, hash_list) { if (stack->hash != hash || stack->size != size) continue; + /* Page owner countable records have a distinct count lifetime. */ + if ((stack->flags ^ flags) & STACK_DEPOT_FLAG_COUNTABLE) + continue; /* * This may race with depot_free_stack() accessing the freelist @@ -1267,6 +1272,9 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, if (WARN_ON(depot_flags & ~STACK_DEPOT_FLAGS_MASK)) return 0; + if (WARN_ON_ONCE((depot_flags & STACK_DEPOT_FLAG_GET) && + (depot_flags & STACK_DEPOT_FLAG_COUNTABLE))) + return 0; /* * If this stack trace is from an interrupt, including anything before @@ -1281,7 +1289,7 @@ depot_stack_handle_t stack_depot_save_flags(unsigned long *entries, if (unlikely(nr_entries == 0) || stack_depot_disabled) return 0; - if (!(depot_flags & STACK_DEPOT_FLAG_GET) && + if (!(depot_flags & (STACK_DEPOT_FLAG_GET | STACK_DEPOT_FLAG_COUNTABLE)) && static_branch_unlikely(&stack_depot_trie_enabled)) { if (nr_entries > CONFIG_STACKDEPOT_MAX_FRAMES) nr_entries = CONFIG_STACKDEPOT_MAX_FRAMES; @@ -1374,12 +1382,20 @@ EXPORT_SYMBOL_GPL(stack_depot_save); struct stack_record *__stack_depot_get_stack_record(depot_stack_handle_t handle) { + struct stack_record *stack; + if (!handle) return NULL; if (WARN_ON_ONCE(stack_depot_handle_is_trie(handle))) return NULL; - return depot_fetch_stack(handle); + stack = depot_fetch_stack(handle); + if (!stack) + return NULL; + if (WARN_ON_ONCE(!(stack->flags & STACK_DEPOT_FLAG_COUNTABLE))) + return NULL; + + return stack; } static void frame_run_init(const unsigned long *entries, @@ -2136,6 +2152,8 @@ void stack_depot_put(depot_stack_handle_t handle) if (WARN(!stack, "corrupt handle or unbalanced stack_depot_put()")) return; + if (WARN_ON_ONCE(stack->flags & STACK_DEPOT_FLAG_COUNTABLE)) + return; if (refcount_dec_and_test(&stack->count)) depot_free_stack(stack); } diff --git a/lib/tests/stackdepot_kunit.c b/lib/tests/stackdepot_kunit.c index 75fa16268c0b..be14cae98fcf 100644 --- a/lib/tests/stackdepot_kunit.c +++ b/lib/tests/stackdepot_kunit.c @@ -167,6 +167,60 @@ static void stackdepot_snprint_public(struct kunit *test) KUNIT_EXPECT_STREQ(test, actual, expected); } +static void stackdepot_countable_public(struct kunit *test) +{ + unsigned long plain_entries[] = { + 0x141000UL, + 0x142000UL, + 0x143000UL, + }; + unsigned long get_entries[] = { + 0x151000UL, + 0x152000UL, + 0x153000UL, + }; + unsigned long fetched[ARRAY_SIZE(plain_entries)] = {}; + depot_flags_t countable = STACK_DEPOT_FLAG_CAN_ALLOC | + STACK_DEPOT_FLAG_COUNTABLE; + struct stack_record *record; + depot_stack_handle_t count_handle; + depot_stack_handle_t plain_handle; + depot_stack_handle_t get_handle; + unsigned int get_nr = ARRAY_SIZE(get_entries); + unsigned int plain_nr = ARRAY_SIZE(plain_entries); + unsigned int nr_entries; + + KUNIT_ASSERT_EQ(test, stack_depot_init(), 0); + + plain_handle = stack_depot_save(plain_entries, plain_nr, GFP_KERNEL); + KUNIT_ASSERT_NE(test, plain_handle, (depot_stack_handle_t)0); + count_handle = stack_depot_save_flags(plain_entries, plain_nr, GFP_KERNEL, + countable); + KUNIT_ASSERT_NE(test, count_handle, (depot_stack_handle_t)0); + record = __stack_depot_get_stack_record(count_handle); + KUNIT_ASSERT_NOT_NULL(test, record); + KUNIT_EXPECT_EQ(test, record->size, (u16)plain_nr); + KUNIT_EXPECT_MEMEQ(test, record->entries, plain_entries, + sizeof(plain_entries)); + nr_entries = stack_depot_fetch_into(count_handle, fetched, + ARRAY_SIZE(fetched)); + KUNIT_EXPECT_EQ(test, nr_entries, plain_nr); + KUNIT_EXPECT_MEMEQ(test, fetched, plain_entries, sizeof(plain_entries)); + + get_handle = stack_depot_save_flags(get_entries, get_nr, GFP_KERNEL, + STACK_DEPOT_FLAG_CAN_ALLOC | + STACK_DEPOT_FLAG_GET); + KUNIT_ASSERT_NE(test, get_handle, (depot_stack_handle_t)0); + count_handle = stack_depot_save_flags(get_entries, get_nr, GFP_KERNEL, + countable); + KUNIT_ASSERT_NE(test, count_handle, (depot_stack_handle_t)0); + record = __stack_depot_get_stack_record(count_handle); + KUNIT_ASSERT_NOT_NULL(test, record); + KUNIT_EXPECT_MEMEQ(test, record->entries, get_entries, sizeof(get_entries)); + + stack_depot_put(get_handle); +} + static void stackdepot_fetch_into_roundtrip(struct kunit *test) { unsigned long entries[] = { @@ -392,6 +446,7 @@ static struct kunit_case stackdepot_test_cases[] = { KUNIT_CASE(stackdepot_trie_max_path_roundtrip), KUNIT_CASE(stackdepot_save_flags_public), KUNIT_CASE(stackdepot_snprint_public), + KUNIT_CASE(stackdepot_countable_public), KUNIT_CASE(stackdepot_fetch_into_roundtrip), KUNIT_CASE(stackdepot_fetch_into_rejects_missing_or_short_stack), KUNIT_CASE(stackdepot_trie_topology_roundtrip), diff --git a/mm/page_owner.c b/mm/page_owner.c index fbbda7ba914b..af37532729b0 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -119,7 +119,8 @@ static __always_inline depot_stack_handle_t create_dummy_stack(void) unsigned int nr_entries; nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0); - return stack_depot_save(entries, nr_entries, GFP_KERNEL); + return stack_depot_save_flags(entries, nr_entries, GFP_KERNEL, + STACK_DEPOT_FLAG_CAN_ALLOC | STACK_DEPOT_FLAG_COUNTABLE); } static noinline void register_dummy_stack(void) @@ -181,7 +182,8 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags) set_current_in_page_owner(); nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 2); - handle = stack_depot_save(entries, nr_entries, flags); + handle = stack_depot_save_flags(entries, nr_entries, flags, + STACK_DEPOT_FLAG_CAN_ALLOC | STACK_DEPOT_FLAG_COUNTABLE); if (!handle) handle = failure_handle; unset_current_in_page_owner(); -- Git-155)