Skip to content

Commit ca8a667

Browse files
committed
Merge tag 'trace-v6.8-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull tracing fixes from Steven Rostedt: - Fix broken direct trampolines being called when another callback is attached the same function. ARM 64 does not support FTRACE_WITH_REGS, and when it added direct trampoline calls from ftrace, it removed the "WITH_REGS" flag from the ftrace_ops for direct trampolines. This broke x86 as x86 requires direct trampolines to have WITH_REGS. This wasn't noticed because direct trampolines work as long as the function it is attached to is not shared with other callbacks (like the function tracer). When there are other callbacks, a helper trampoline is called, to call all the non direct callbacks and when it returns, the direct trampoline is called. For x86, the direct trampoline sets a flag in the regs field to tell the x86 specific code to call the direct trampoline. But this only works if the ftrace_ops had WITH_REGS set. ARM does things differently that does not require this. For now, set WITH_REGS if the arch supports WITH_REGS (which ARM does not), and this makes it work for both ARM64 and x86. - Fix wasted memory in the saved_cmdlines logic. The saved_cmdlines is a cache that maps PIDs to COMMs that tracing can use. Most trace events only save the PID in the event. The saved_cmdlines file lists PIDs to COMMs so that the tracing tools can show an actual name and not just a PID for each event. There's an array of PIDs that map to a small set of saved COMM strings. The array is set to PID_MAX_DEFAULT which is usually set to 32768. When a PID comes in, it will add itself to this array along with the index into the COMM array (note if the system allows more than PID_MAX_DEFAULT, this cache is similar to cache lines as an update of a PID that has the same PID_MAX_DEFAULT bits set will flush out another task with the same matching bits set). A while ago, the size of this cache was changed to be dynamic and the array was moved into a structure and created with kmalloc(). But this new structure had the size of 131104 bytes, or 0x20020 in hex. As kmalloc allocates in powers of two, it was actually allocating 0x40000 bytes (262144) leaving 131040 bytes of wasted memory. The last element of this structure was a pointer to the COMM string array which defaulted to just saving 128 COMMs. By changing the last field of this structure to a variable length string, and just having it round up to fill the allocated memory, the default size of the saved COMM cache is now 8190. This not only uses the wasted space, but actually saves space by removing the extra allocation for the COMM names. * tag 'trace-v6.8-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: tracing: Fix wasted memory in saved_cmdlines logic ftrace: Fix DIRECT_CALLS to use SAVE_REGS by default
2 parents 6dc512a + 44dc5c4 commit ca8a667

File tree

2 files changed

+47
-38
lines changed

2 files changed

+47
-38
lines changed

kernel/trace/ftrace.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5325,7 +5325,17 @@ static LIST_HEAD(ftrace_direct_funcs);
53255325

53265326
static int register_ftrace_function_nolock(struct ftrace_ops *ops);
53275327

5328+
/*
5329+
* If there are multiple ftrace_ops, use SAVE_REGS by default, so that direct
5330+
* call will be jumped from ftrace_regs_caller. Only if the architecture does
5331+
* not support ftrace_regs_caller but direct_call, use SAVE_ARGS so that it
5332+
* jumps from ftrace_caller for multiple ftrace_ops.
5333+
*/
5334+
#ifndef HAVE_DYNAMIC_FTRACE_WITH_REGS
53285335
#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_ARGS)
5336+
#else
5337+
#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS)
5338+
#endif
53295339

53305340
static int check_direct_multi(struct ftrace_ops *ops)
53315341
{

kernel/trace/trace.c

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2320,7 +2320,7 @@ struct saved_cmdlines_buffer {
23202320
unsigned *map_cmdline_to_pid;
23212321
unsigned cmdline_num;
23222322
int cmdline_idx;
2323-
char *saved_cmdlines;
2323+
char saved_cmdlines[];
23242324
};
23252325
static struct saved_cmdlines_buffer *savedcmd;
23262326

@@ -2334,47 +2334,58 @@ static inline void set_cmdline(int idx, const char *cmdline)
23342334
strncpy(get_saved_cmdlines(idx), cmdline, TASK_COMM_LEN);
23352335
}
23362336

2337-
static int allocate_cmdlines_buffer(unsigned int val,
2338-
struct saved_cmdlines_buffer *s)
2337+
static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)
2338+
{
2339+
int order = get_order(sizeof(*s) + s->cmdline_num * TASK_COMM_LEN);
2340+
2341+
kfree(s->map_cmdline_to_pid);
2342+
free_pages((unsigned long)s, order);
2343+
}
2344+
2345+
static struct saved_cmdlines_buffer *allocate_cmdlines_buffer(unsigned int val)
23392346
{
2347+
struct saved_cmdlines_buffer *s;
2348+
struct page *page;
2349+
int orig_size, size;
2350+
int order;
2351+
2352+
/* Figure out how much is needed to hold the given number of cmdlines */
2353+
orig_size = sizeof(*s) + val * TASK_COMM_LEN;
2354+
order = get_order(orig_size);
2355+
size = 1 << (order + PAGE_SHIFT);
2356+
page = alloc_pages(GFP_KERNEL, order);
2357+
if (!page)
2358+
return NULL;
2359+
2360+
s = page_address(page);
2361+
memset(s, 0, sizeof(*s));
2362+
2363+
/* Round up to actual allocation */
2364+
val = (size - sizeof(*s)) / TASK_COMM_LEN;
2365+
s->cmdline_num = val;
2366+
23402367
s->map_cmdline_to_pid = kmalloc_array(val,
23412368
sizeof(*s->map_cmdline_to_pid),
23422369
GFP_KERNEL);
2343-
if (!s->map_cmdline_to_pid)
2344-
return -ENOMEM;
2345-
2346-
s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
2347-
if (!s->saved_cmdlines) {
2348-
kfree(s->map_cmdline_to_pid);
2349-
return -ENOMEM;
2370+
if (!s->map_cmdline_to_pid) {
2371+
free_saved_cmdlines_buffer(s);
2372+
return NULL;
23502373
}
23512374

23522375
s->cmdline_idx = 0;
2353-
s->cmdline_num = val;
23542376
memset(&s->map_pid_to_cmdline, NO_CMDLINE_MAP,
23552377
sizeof(s->map_pid_to_cmdline));
23562378
memset(s->map_cmdline_to_pid, NO_CMDLINE_MAP,
23572379
val * sizeof(*s->map_cmdline_to_pid));
23582380

2359-
return 0;
2381+
return s;
23602382
}
23612383

23622384
static int trace_create_savedcmd(void)
23632385
{
2364-
int ret;
2365-
2366-
savedcmd = kmalloc(sizeof(*savedcmd), GFP_KERNEL);
2367-
if (!savedcmd)
2368-
return -ENOMEM;
2369-
2370-
ret = allocate_cmdlines_buffer(SAVED_CMDLINES_DEFAULT, savedcmd);
2371-
if (ret < 0) {
2372-
kfree(savedcmd);
2373-
savedcmd = NULL;
2374-
return -ENOMEM;
2375-
}
2386+
savedcmd = allocate_cmdlines_buffer(SAVED_CMDLINES_DEFAULT);
23762387

2377-
return 0;
2388+
return savedcmd ? 0 : -ENOMEM;
23782389
}
23792390

23802391
int is_tracing_stopped(void)
@@ -6056,26 +6067,14 @@ tracing_saved_cmdlines_size_read(struct file *filp, char __user *ubuf,
60566067
return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
60576068
}
60586069

6059-
static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)
6060-
{
6061-
kfree(s->saved_cmdlines);
6062-
kfree(s->map_cmdline_to_pid);
6063-
kfree(s);
6064-
}
6065-
60666070
static int tracing_resize_saved_cmdlines(unsigned int val)
60676071
{
60686072
struct saved_cmdlines_buffer *s, *savedcmd_temp;
60696073

6070-
s = kmalloc(sizeof(*s), GFP_KERNEL);
6074+
s = allocate_cmdlines_buffer(val);
60716075
if (!s)
60726076
return -ENOMEM;
60736077

6074-
if (allocate_cmdlines_buffer(val, s) < 0) {
6075-
kfree(s);
6076-
return -ENOMEM;
6077-
}
6078-
60796078
preempt_disable();
60806079
arch_spin_lock(&trace_cmdline_lock);
60816080
savedcmd_temp = savedcmd;

0 commit comments

Comments
 (0)