Revert "mm/slub: use stackdepot to save stack trace in objects"

This reverts commit 788691464c29455346dc613a3b43c2fb9e5757a4.

It's not clear why, but it causes unexplained problems in entirely
unrelated xfs code. The most likely explanation is some slab
corruption, possibly triggered due to CONFIG_SLUB_DEBUG_ON. See [1].

It ends up having a few other problems too, like build errors on
arch/arc, and Geert reporting it using much more memory on m68k [3] (it
probably does so elsewhere too, but it is probably just more noticeable
on m68k).

The architecture issues (both build and memory use) are likely just
because this change effectively force-enabled STACKDEPOT (along with a
very bad default value for the stackdepot hash size). But together with
the xfs issue, this all smells like "this commit was not ready" to me.

Link: https://lore.kernel.org/linux-xfs/YPE3l82acwgI2OiV@infradead.org/ [1]
Link: https://lore.kernel.org/lkml/202107150600.LkGNb4Vb-lkp@intel.com/ [2]
Link: https://lore.kernel.org/lkml/CAMuHMdW=eoVzM1Re5FVoEN87nKfiLmM2+Ah7eNu2KXEhCvbZyA@mail.gmail.com/ [3]
Reported-by: Christoph Hellwig <hch@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Linus Torvalds 4 years ago ae14c63a 5d766d55

+30 -50

2 changed files

expand all

init

Kconfig

slub.c

-1

init/Kconfig

··· 1847 1847 default y 1848 1848 bool "Enable SLUB debugging support" if EXPERT 1849 1849 depends on SLUB && SYSFS 1850 - select STACKDEPOT if STACKTRACE_SUPPORT 1851 1850 help 1852 1851 SLUB has extensive debug support features. Disabling these can 1853 1852 result in significant savings in code size. This also disables

+30 -49

mm/slub.c

··· 26 26 #include <linux/cpuset.h> 27 27 #include <linux/mempolicy.h> 28 28 #include <linux/ctype.h> 29 - #include <linux/stackdepot.h> 30 29 #include <linux/debugobjects.h> 31 30 #include <linux/kallsyms.h> 32 31 #include <linux/kfence.h> ··· 206 207 #define TRACK_ADDRS_COUNT 16 207 208 struct track { 208 209 unsigned long addr; /* Called from address */ 209 - #ifdef CONFIG_STACKDEPOT 210 - depot_stack_handle_t handle; 210 + #ifdef CONFIG_STACKTRACE 211 + unsigned long addrs[TRACK_ADDRS_COUNT]; /* Called from address */ 211 212 #endif 212 213 int cpu; /* Was running on cpu */ 213 214 int pid; /* Pid context */ ··· 611 612 return kasan_reset_tag(p + alloc); 612 613 } 613 614 614 - #ifdef CONFIG_STACKDEPOT 615 - static depot_stack_handle_t save_stack_depot_trace(gfp_t flags) 616 - { 617 - unsigned long entries[TRACK_ADDRS_COUNT]; 618 - depot_stack_handle_t handle; 619 - unsigned int nr_entries; 620 - 621 - nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 4); 622 - handle = stack_depot_save(entries, nr_entries, flags); 623 - return handle; 624 - } 625 - #endif 626 - 627 615 static void set_track(struct kmem_cache *s, void *object, 628 616 enum track_item alloc, unsigned long addr) 629 617 { 630 618 struct track *p = get_track(s, object, alloc); 631 619 632 620 if (addr) { 633 - #ifdef CONFIG_STACKDEPOT 634 - p->handle = save_stack_depot_trace(GFP_NOWAIT); 621 + #ifdef CONFIG_STACKTRACE 622 + unsigned int nr_entries; 623 + 624 + metadata_access_enable(); 625 + nr_entries = stack_trace_save(kasan_reset_tag(p->addrs), 626 + TRACK_ADDRS_COUNT, 3); 627 + metadata_access_disable(); 628 + 629 + if (nr_entries < TRACK_ADDRS_COUNT) 630 + p->addrs[nr_entries] = 0; 635 631 #endif 636 632 p->addr = addr; 637 633 p->cpu = smp_processor_id(); ··· 653 659 654 660 pr_err("%s in %pS age=%lu cpu=%u pid=%d\n", 655 661 s, (void *)t->addr, pr_time - t->when, t->cpu, t->pid); 656 - #ifdef CONFIG_STACKDEPOT 662 + #ifdef CONFIG_STACKTRACE 657 663 { 658 - depot_stack_handle_t handle; 659 - unsigned long *entries; 660 - unsigned int nr_entries; 661 - 662 - handle = READ_ONCE(t->handle); 663 - if (!handle) { 664 - pr_err("object allocation/free stack trace missing\n"); 665 - } else { 666 - nr_entries = stack_depot_fetch(handle, &entries); 667 - stack_trace_print(entries, nr_entries, 0); 668 - } 664 + int i; 665 + for (i = 0; i < TRACK_ADDRS_COUNT; i++) 666 + if (t->addrs[i]) 667 + pr_err("\t%pS\n", (void *)t->addrs[i]); 668 + else 669 + break; 669 670 } 670 671 #endif 671 672 } ··· 4034 4045 objp = fixup_red_left(s, objp); 4035 4046 trackp = get_track(s, objp, TRACK_ALLOC); 4036 4047 kpp->kp_ret = (void *)trackp->addr; 4037 - #ifdef CONFIG_STACKDEPOT 4038 - { 4039 - depot_stack_handle_t handle; 4040 - unsigned long *entries; 4041 - unsigned int nr_entries; 4048 + #ifdef CONFIG_STACKTRACE 4049 + for (i = 0; i < KS_ADDRS_COUNT && i < TRACK_ADDRS_COUNT; i++) { 4050 + kpp->kp_stack[i] = (void *)trackp->addrs[i]; 4051 + if (!kpp->kp_stack[i]) 4052 + break; 4053 + } 4042 4054 4043 - handle = READ_ONCE(trackp->handle); 4044 - if (handle) { 4045 - nr_entries = stack_depot_fetch(handle, &entries); 4046 - for (i = 0; i < KS_ADDRS_COUNT && i < nr_entries; i++) 4047 - kpp->kp_stack[i] = (void *)entries[i]; 4048 - } 4049 - 4050 - trackp = get_track(s, objp, TRACK_FREE); 4051 - handle = READ_ONCE(trackp->handle); 4052 - if (handle) { 4053 - nr_entries = stack_depot_fetch(handle, &entries); 4054 - for (i = 0; i < KS_ADDRS_COUNT && i < nr_entries; i++) 4055 - kpp->kp_free_stack[i] = (void *)entries[i]; 4056 - } 4055 + trackp = get_track(s, objp, TRACK_FREE); 4056 + for (i = 0; i < KS_ADDRS_COUNT && i < TRACK_ADDRS_COUNT; i++) { 4057 + kpp->kp_free_stack[i] = (void *)trackp->addrs[i]; 4058 + if (!kpp->kp_free_stack[i]) 4059 + break; 4057 4060 } 4058 4061 #endif 4059 4062 #endif

Configure Feed

Configure Feed