* [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls @ 2019-10-22 18:24 Hassan Naveed 2019-10-22 19:51 ` Steven Rostedt 0 siblings, 1 reply; 6+ messages in thread From: Hassan Naveed @ 2019-10-22 18:24 UTC (permalink / raw) To: Steven Rostedt, Ingo Molnar, linux-kernel; +Cc: Paul Burton, Hassan Naveed From: Hassan Naveed <hnaveed@wavecomp.com> Currently, a lot of memory is wasted for architectures like MIPS when init_ftrace_syscalls() allocates the array for syscalls using kcalloc. This is because syscalls numbers start from 4000, 5000 or 6000 and array elements up to that point are unused. Fix this by using a data structure more suited to storing sparsely populated arrays. The XARRAY data structure, implemented using radix trees, is much more memory efficient for storing the syscalls in question. Signed-off-by: Hassan Naveed <hnaveed@wavecomp.com> --- kernel/trace/trace_syscalls.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index f93a56d2db27..1fee710be874 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -7,6 +7,7 @@ #include <linux/module.h> /* for MODULE_NAME_LEN via KSYM_SYMBOL_LEN */ #include <linux/ftrace.h> #include <linux/perf_event.h> +#include <linux/xarray.h> #include <asm/syscall.h> #include "trace_output.h" @@ -30,7 +31,7 @@ syscall_get_enter_fields(struct trace_event_call *call) extern struct syscall_metadata *__start_syscalls_metadata[]; extern struct syscall_metadata *__stop_syscalls_metadata[]; -static struct syscall_metadata **syscalls_metadata; +static DEFINE_XARRAY(syscalls_metadata); #ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME static inline bool arch_syscall_match_sym_name(const char *sym, const char *name) @@ -101,10 +102,7 @@ find_syscall_meta(unsigned long syscall) static struct syscall_metadata *syscall_nr_to_meta(int nr) { - if (!syscalls_metadata || nr >= NR_syscalls || nr < 0) - return NULL; - - return syscalls_metadata[nr]; + return xa_load(&syscalls_metadata, (unsigned long)nr); } const char *get_syscall_name(int syscall) @@ -535,13 +533,6 @@ void __init init_ftrace_syscalls(void) unsigned long addr; int i; - syscalls_metadata = kcalloc(NR_syscalls, sizeof(*syscalls_metadata), - GFP_KERNEL); - if (!syscalls_metadata) { - WARN_ON(1); - return; - } - for (i = 0; i < NR_syscalls; i++) { addr = arch_syscall_addr(i); meta = find_syscall_meta(addr); @@ -549,7 +540,7 @@ void __init init_ftrace_syscalls(void) continue; meta->syscall_nr = i; - syscalls_metadata[i] = meta; + xa_store(&syscalls_metadata, i, meta, GFP_KERNEL); } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls 2019-10-22 18:24 [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls Hassan Naveed @ 2019-10-22 19:51 ` Steven Rostedt 2019-10-22 21:45 ` Steven Rostedt 0 siblings, 1 reply; 6+ messages in thread From: Steven Rostedt @ 2019-10-22 19:51 UTC (permalink / raw) To: Hassan Naveed; +Cc: Ingo Molnar, linux-kernel, Paul Burton On Tue, 22 Oct 2019 18:24:25 +0000 Hassan Naveed <hnaveed@wavecomp.com> wrote: Nit, the subject should simply be: "tracing: Use xarray for syscall trace events" > Signed-off-by: Hassan Naveed <hnaveed@wavecomp.com> > --- > kernel/trace/trace_syscalls.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > index f93a56d2db27..1fee710be874 100644 > --- a/kernel/trace/trace_syscalls.c > +++ b/kernel/trace/trace_syscalls.c > @@ -7,6 +7,7 @@ > #include <linux/module.h> /* for MODULE_NAME_LEN via KSYM_SYMBOL_LEN */ > #include <linux/ftrace.h> > #include <linux/perf_event.h> > +#include <linux/xarray.h> > #include <asm/syscall.h> > > #include "trace_output.h" > @@ -30,7 +31,7 @@ syscall_get_enter_fields(struct trace_event_call *call) > extern struct syscall_metadata *__start_syscalls_metadata[]; > extern struct syscall_metadata *__stop_syscalls_metadata[]; > > -static struct syscall_metadata **syscalls_metadata; > +static DEFINE_XARRAY(syscalls_metadata); > > #ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME > static inline bool arch_syscall_match_sym_name(const char *sym, const char *name) > @@ -101,10 +102,7 @@ find_syscall_meta(unsigned long syscall) > > static struct syscall_metadata *syscall_nr_to_meta(int nr) > { > - if (!syscalls_metadata || nr >= NR_syscalls || nr < 0) > - return NULL; > - > - return syscalls_metadata[nr]; > + return xa_load(&syscalls_metadata, (unsigned long)nr); > } > > const char *get_syscall_name(int syscall) > @@ -535,13 +533,6 @@ void __init init_ftrace_syscalls(void) > unsigned long addr; > int i; > > - syscalls_metadata = kcalloc(NR_syscalls, sizeof(*syscalls_metadata), > - GFP_KERNEL); > - if (!syscalls_metadata) { > - WARN_ON(1); > - return; > - } > - > for (i = 0; i < NR_syscalls; i++) { > addr = arch_syscall_addr(i); > meta = find_syscall_meta(addr); > @@ -549,7 +540,7 @@ void __init init_ftrace_syscalls(void) > continue; > > meta->syscall_nr = i; > - syscalls_metadata[i] = meta; > + xa_store(&syscalls_metadata, i, meta, GFP_KERNEL); Shouldn't xa_store() return be tested for memory failure? -- Steve > } > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls 2019-10-22 19:51 ` Steven Rostedt @ 2019-10-22 21:45 ` Steven Rostedt 2019-11-13 19:24 ` Steven Rostedt ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Steven Rostedt @ 2019-10-22 21:45 UTC (permalink / raw) To: Hassan Naveed; +Cc: Ingo Molnar, linux-kernel, Paul Burton On Tue, 22 Oct 2019 15:51:04 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > static struct syscall_metadata *syscall_nr_to_meta(int nr) > > { > > - if (!syscalls_metadata || nr >= NR_syscalls || nr < 0) > > - return NULL; > > - > > - return syscalls_metadata[nr]; > > + return xa_load(&syscalls_metadata, (unsigned long)nr); > > } > > There appears to be a slight overhead to this for archs that do not have a sparse syscall array. I wonder if we should make this only applicable for archs (via adding a HAVE_SPARSE_SYSCALL_NR define and checking against it). Then if an arch doesn't have a sparse array of system calls, it uses a normal lookup, but for archs that do, it can define this for this type of lookup. There's not much to this patch, so it wouldn't be too difficult to support both methods. Without this patch I ran: # trace-cmd start -e syscalls # /work/c/hackbench 50 Time: 15.702 # /work/c/hackbench 50 Time: 15.932 # /work/c/hackbench 50 Time: 15.893 # /work/c/hackbench 50 Time: 16.038 # /work/c/hackbench 50 Time: 15.429 With the patch it had: # trace-cmd start -e syscalls # /work/c/hackbench 50 Time: 16.582 # /work/c/hackbench 50 Time: 15.972 # /work/c/hackbench 50 Time: 16.078 # /work/c/hackbench 50 Time: 16.133 # /work/c/hackbench 50 Time: 16.263 -- Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls 2019-10-22 21:45 ` Steven Rostedt @ 2019-11-13 19:24 ` Steven Rostedt 2019-11-15 23:44 ` [PATCH v2 1/2] tracing: Use xarray for syscall trace events Hassan Naveed 2019-11-15 23:44 ` [PATCH v2 2/2] tracing: enable syscall optimization for MIPS Hassan Naveed 2 siblings, 0 replies; 6+ messages in thread From: Steven Rostedt @ 2019-11-13 19:24 UTC (permalink / raw) To: Hassan Naveed; +Cc: Ingo Molnar, linux-kernel, Paul Burton ping? -- Steve On Tue, 22 Oct 2019 17:45:51 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 22 Oct 2019 15:51:04 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > static struct syscall_metadata *syscall_nr_to_meta(int nr) > > > { > > > - if (!syscalls_metadata || nr >= NR_syscalls || nr < 0) > > > - return NULL; > > > - > > > - return syscalls_metadata[nr]; > > > + return xa_load(&syscalls_metadata, (unsigned long)nr); > > > } > > > > > There appears to be a slight overhead to this for archs that do not > have a sparse syscall array. I wonder if we should make this only > applicable for archs (via adding a HAVE_SPARSE_SYSCALL_NR define and > checking against it). Then if an arch doesn't have a sparse array of > system calls, it uses a normal lookup, but for archs that do, it can > define this for this type of lookup. > > There's not much to this patch, so it wouldn't be too difficult to > support both methods. > > Without this patch I ran: > > # trace-cmd start -e syscalls > # /work/c/hackbench 50 > Time: 15.702 > # /work/c/hackbench 50 > Time: 15.932 > # /work/c/hackbench 50 > Time: 15.893 > # /work/c/hackbench 50 > Time: 16.038 > # /work/c/hackbench 50 > Time: 15.429 > > > With the patch it had: > > # trace-cmd start -e syscalls > # /work/c/hackbench 50 > Time: 16.582 > # /work/c/hackbench 50 > Time: 15.972 > # /work/c/hackbench 50 > Time: 16.078 > # /work/c/hackbench 50 > Time: 16.133 > # /work/c/hackbench 50 > Time: 16.263 > > -- Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] tracing: Use xarray for syscall trace events 2019-10-22 21:45 ` Steven Rostedt 2019-11-13 19:24 ` Steven Rostedt @ 2019-11-15 23:44 ` Hassan Naveed 2019-11-15 23:44 ` [PATCH v2 2/2] tracing: enable syscall optimization for MIPS Hassan Naveed 2 siblings, 0 replies; 6+ messages in thread From: Hassan Naveed @ 2019-11-15 23:44 UTC (permalink / raw) Cc: paulburton, Hassan Naveed, Ralf Baechle, Paul Burton, James Hogan, Steven Rostedt, Ingo Molnar, Arnd Bergmann, Yury Norov, Geert Uytterhoeven, Michael Ellerman, Finn Thain, Ard Biesheuvel, Masahiro Yamada, Joel Fernandes (Google), linux-kernel, linux-mips Currently, a lot of memory is wasted for architectures like MIPS when init_ftrace_syscalls() allocates the array for syscalls using kcalloc. This is because syscalls numbers start from 4000, 5000 or 6000 and array elements up to that point are unused. Fix this by using a data structure more suited to storing sparsely populated arrays. The XARRAY data structure, implemented using radix trees, is much more memory efficient for storing the syscalls in question. Signed-off-by: Hassan Naveed <hnaveed@wavecomp.com> Reviewed-by: Paul Burton <paulburton@kernel.org> --- Changes in v2: - New commit message - Added HAVE_SPARSE_SYSCALL_NR to arch/Kconfig for archs with sparse syscall arrays - Changes split up into two patches; one that provides xarray support and the second in which MIPS selects it --- arch/Kconfig | 8 ++++++++ kernel/trace/trace_syscalls.c | 32 +++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 33687dddd86a..e3783d94f919 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -901,6 +901,14 @@ config HAVE_ARCH_PREL32_RELOCATIONS config ARCH_USE_MEMREMAP_PROT bool +config HAVE_SPARSE_SYSCALL_NR + bool + help + An architecture should select this if its syscall numbering is sparse + to save space. For example, MIPS architecture has a syscall array with + entries at 4000, 5000 and 6000 locations. This option turns on syscall + related optimizations for a given architecture. + source "kernel/gcov/Kconfig" source "scripts/gcc-plugins/Kconfig" diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index f93a56d2db27..ffff52399d97 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -7,6 +7,7 @@ #include <linux/module.h> /* for MODULE_NAME_LEN via KSYM_SYMBOL_LEN */ #include <linux/ftrace.h> #include <linux/perf_event.h> +#include <linux/xarray.h> #include <asm/syscall.h> #include "trace_output.h" @@ -30,6 +31,7 @@ syscall_get_enter_fields(struct trace_event_call *call) extern struct syscall_metadata *__start_syscalls_metadata[]; extern struct syscall_metadata *__stop_syscalls_metadata[]; +static DEFINE_XARRAY(syscalls_metadata_sparse); static struct syscall_metadata **syscalls_metadata; #ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME @@ -101,6 +103,9 @@ find_syscall_meta(unsigned long syscall) static struct syscall_metadata *syscall_nr_to_meta(int nr) { + if (IS_ENABLED(CONFIG_HAVE_SPARSE_SYSCALL_NR)) + return xa_load(&syscalls_metadata_sparse, (unsigned long)nr); + if (!syscalls_metadata || nr >= NR_syscalls || nr < 0) return NULL; @@ -534,12 +539,16 @@ void __init init_ftrace_syscalls(void) struct syscall_metadata *meta; unsigned long addr; int i; - - syscalls_metadata = kcalloc(NR_syscalls, sizeof(*syscalls_metadata), - GFP_KERNEL); - if (!syscalls_metadata) { - WARN_ON(1); - return; + void *ret; + + if (!IS_ENABLED(CONFIG_HAVE_SPARSE_SYSCALL_NR)) { + syscalls_metadata = kcalloc(NR_syscalls, + sizeof(*syscalls_metadata), + GFP_KERNEL); + if (!syscalls_metadata) { + WARN_ON(1); + return; + } } for (i = 0; i < NR_syscalls; i++) { @@ -549,7 +558,16 @@ void __init init_ftrace_syscalls(void) continue; meta->syscall_nr = i; - syscalls_metadata[i] = meta; + + if (!IS_ENABLED(CONFIG_HAVE_SPARSE_SYSCALL_NR)) { + syscalls_metadata[i] = meta; + } else { + ret = xa_store(&syscalls_metadata_sparse, i, meta, + GFP_KERNEL); + WARN(xa_is_err(ret), + "Syscall memory allocation failed\n"); + } + } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] tracing: enable syscall optimization for MIPS 2019-10-22 21:45 ` Steven Rostedt 2019-11-13 19:24 ` Steven Rostedt 2019-11-15 23:44 ` [PATCH v2 1/2] tracing: Use xarray for syscall trace events Hassan Naveed @ 2019-11-15 23:44 ` Hassan Naveed 2 siblings, 0 replies; 6+ messages in thread From: Hassan Naveed @ 2019-11-15 23:44 UTC (permalink / raw) Cc: paulburton, Hassan Naveed, Ralf Baechle, Paul Burton, James Hogan, Steven Rostedt, Ingo Molnar, Arnd Bergmann, Yury Norov, Geert Uytterhoeven, Michael Ellerman, Ard Biesheuvel, Masahiro Yamada, Joel Fernandes (Google), Finn Thain, linux-kernel, linux-mips Since MIPS architecture has a sparse syscall array, select the HAVE_SPARSE_SYSCALL_NR to save space. Signed-off-by: Hassan Naveed <hnaveed@wavecomp.com> Reviewed-by: Paul Burton <paulburton@kernel.org> --- arch/mips/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 4a5f5b0ee9a9..32421ecff933 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -71,6 +71,7 @@ config MIPS select HAVE_PERF_EVENTS select HAVE_REGS_AND_STACK_ACCESS_API select HAVE_RSEQ + select HAVE_SPARSE_SYSCALL_NR select HAVE_STACKPROTECTOR select HAVE_SYSCALL_TRACEPOINTS select HAVE_VIRT_CPU_ACCOUNTING_GEN if 64BIT || !SMP -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-15 23:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-22 18:24 [PATCH] TRACING: FTRACE: Use xarray structure for ftrace syscalls Hassan Naveed 2019-10-22 19:51 ` Steven Rostedt 2019-10-22 21:45 ` Steven Rostedt 2019-11-13 19:24 ` Steven Rostedt 2019-11-15 23:44 ` [PATCH v2 1/2] tracing: Use xarray for syscall trace events Hassan Naveed 2019-11-15 23:44 ` [PATCH v2 2/2] tracing: enable syscall optimization for MIPS Hassan Naveed
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.