On Fri, 24 May 2024 22:36:52 -0400 Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > [ > Resend for some of you as I missed a comma in the Cc and > quilt died sending this out. > ] > > This is a continuation of the function graph multi user code. > I wrote a proof of concept back in 2019 of this code[1] and > Masami started cleaning it up. I started from Masami's work v10 > that can be found here: > > https://lore.kernel.org/linux-trace-kernel/171509088006.162236.7227326999861366050.stgit@devnote2/ > > This is *only* the code that allows multiple users of function > graph tracing. This is not the fprobe work that Masami is working > to add on top of it. As Masami took my proof of concept, there > was still several things I disliked about that code. Instead of > having Masami clean it up even more, I decided to take over on just > my code and change it up a bit. > > The biggest changes from where Masami left off is mostly renaming more > variables, macros, and function names. I fixed up the current comments > and added more to make the code a bit more understandable. > > At the end of the series, I added two patches to optimize the entry > and exit. On entry, there was a loop that iterated the 16 elements > of the fgraph_array[] looking for any that may have a gops registered > to it. It's quite a waste to do that loop if there's only one > registered user. To fix that, I added a fgraph_array_bitmask that has > its bits set that correspond to the elements of the array. Then > a simple for_each_set_bit() is used for the iteration. I do the same > thing at the exit callback of the function where it iterates over the > bits of the bitmap saved on the ret_stack. > > I also noticed that Masami added code to handle tail calls in the > unwinder and had that in one of my patches. I took that code out > and made it a separate patch with Masami as the author. > > The diff between this and Masami's last update is at the end of this email. Thanks for update the patches! I think your changes are good. I just have some comments and replied. So, the plan is to push this series in the tracing/for-next? I will port my fprobe part on it and run some tests. Thank you, > > Based on Linus commit: 0eb03c7e8e2a4cc3653eb5eeb2d2001182071215 > > [1] https://lore.kernel.org/all/20190525031633.811342628@xxxxxxxxxxx/ > > Masami Hiramatsu (Google) (3): > function_graph: Handle tail calls for stack unwinding > function_graph: Use a simple LRU for fgraph_array index number > ftrace: Add multiple fgraph storage selftest > > Steven Rostedt (Google) (2): > function_graph: Use for_each_set_bit() in __ftrace_return_to_handler() > function_graph: Use bitmask to loop on fgraph entry > > Steven Rostedt (VMware) (15): > function_graph: Convert ret_stack to a series of longs > fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long > function_graph: Add an array structure that will allow multiple callbacks > function_graph: Allow multiple users to attach to function graph > function_graph: Remove logic around ftrace_graph_entry and return > ftrace/function_graph: Pass fgraph_ops to function graph callbacks > ftrace: Allow function_graph tracer to be enabled in instances > ftrace: Allow ftrace startup flags to exist without dynamic ftrace > function_graph: Have the instances use their own ftrace_ops for filtering > function_graph: Add "task variables" per task for fgraph_ops > function_graph: Move set_graph_function tests to shadow stack global var > function_graph: Move graph depth stored data to shadow stack global var > function_graph: Move graph notrace bit to shadow stack global var > function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data() > function_graph: Add selftest for passing local variables > > ---- > arch/arm64/kernel/ftrace.c | 21 +- > arch/loongarch/kernel/ftrace_dyn.c | 15 +- > arch/powerpc/kernel/trace/ftrace.c | 3 +- > arch/riscv/kernel/ftrace.c | 15 +- > arch/x86/kernel/ftrace.c | 19 +- > include/linux/ftrace.h | 42 +- > include/linux/sched.h | 2 +- > include/linux/trace_recursion.h | 39 -- > kernel/trace/fgraph.c | 994 ++++++++++++++++++++++++++++------- > kernel/trace/ftrace.c | 11 +- > kernel/trace/ftrace_internal.h | 2 - > kernel/trace/trace.h | 94 +++- > kernel/trace/trace_functions.c | 8 + > kernel/trace/trace_functions_graph.c | 96 ++-- > kernel/trace/trace_irqsoff.c | 10 +- > kernel/trace/trace_sched_wakeup.c | 10 +- > kernel/trace/trace_selftest.c | 259 ++++++++- > 17 files changed, 1330 insertions(+), 310 deletions(-) > > > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index 3313e4b83aa2..1aae521e5997 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -18,29 +18,36 @@ > #include "ftrace_internal.h" > #include "trace.h" > > -#define FGRAPH_FRAME_SIZE sizeof(struct ftrace_ret_stack) > -#define FGRAPH_FRAME_OFFSET DIV_ROUND_UP(FGRAPH_FRAME_SIZE, sizeof(long)) > > /* > - * On entry to a function (via function_graph_enter()), a new ftrace_ret_stack > - * is allocated on the task's ret_stack with bitmap entry, then each > - * fgraph_ops on the fgraph_array[]'s entryfunc is called and if that returns > - * non-zero, the index into the fgraph_array[] for that fgraph_ops is recorded > - * on the bitmap entry as a bit flag. > - * > - * The top of the ret_stack (when not empty) will always have a reference > - * to the last ftrace_ret_stack saved. All references to the > - * ftrace_ret_stack has the format of: > + * FGRAPH_FRAME_SIZE: Size in bytes of the meta data on the shadow stack > + * FGRAPH_FRAME_OFFSET: Size in long words of the meta data frame > + */ > +#define FGRAPH_FRAME_SIZE sizeof(struct ftrace_ret_stack) > +#define FGRAPH_FRAME_OFFSET DIV_ROUND_UP(FGRAPH_FRAME_SIZE, sizeof(long)) > + > +/* > + * On entry to a function (via function_graph_enter()), a new fgraph frame > + * (ftrace_ret_stack) is pushed onto the stack as well as a word that > + * holds a bitmask and a type (called "bitmap"). The bitmap is defined as: > * > * bits: 0 - 9 offset in words from the previous ftrace_ret_stack > - * (bitmap type should have FGRAPH_FRAME_OFFSET always) > + * > * bits: 10 - 11 Type of storage > * 0 - reserved > * 1 - bitmap of fgraph_array index > * 2 - reserved data > * > - * For bitmap of fgraph_array index > + * For type with "bitmap of fgraph_array index" (FGRAPH_TYPE_BITMAP): > * bits: 12 - 27 The bitmap of fgraph_ops fgraph_array index > + * That is, it's a bitmask of 0-15 (16 bits) > + * where if a corresponding ops in the fgraph_array[] > + * expects a callback from the return of the function > + * it's corresponding bit will be set. > + * > + * > + * The top of the ret_stack (when not empty) will always have a reference > + * word that points to the last fgraph frame that was saved. > * > * For reserved data: > * bits: 12 - 17 The size in words that is stored > @@ -52,7 +59,7 @@ > * stored two words of data, this is what will be on the task's shadow > * ret_stack: (the stack grows upward) > * > - * ret_stack[SHADOW_STACK_IN_WORD] > + * ret_stack[SHADOW_STACK_OFFSET] > * | SHADOW_STACK_TASK_VARS(ret_stack)[15] | > * ... > * | SHADOW_STACK_TASK_VARS(ret_stack)[0] | > @@ -60,6 +67,8 @@ > * ... > * | | <- task->curr_ret_stack > * +--------------------------------------------+ > + * | (3 << 12) | (3 << 10) | FGRAPH_FRAME_OFFSET| > + * | *or put another way* | > * | (3 << FGRAPH_DATA_INDEX_SHIFT)| \ | This is for fgraph_ops[3]. > * | ((2 - 1) << FGRAPH_DATA_SHIFT)| \ | The data size is 2 words. > * | (FGRAPH_TYPE_DATA << FGRAPH_TYPE_SHIFT)| \ | > @@ -67,7 +76,9 @@ > * +--------------------------------------------+ ( It is 4 words from the ret_stack) > * | STORED DATA WORD 2 | > * | STORED DATA WORD 1 | > - * +------i-------------------------------------+ > + * +--------------------------------------------+ > + * | (9 << 12) | (1 << 10) | FGRAPH_FRAME_OFFSET| > + * | *or put another way* | > * | (BIT(3)|BIT(0)) << FGRAPH_INDEX_SHIFT | \ | > * | FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT| \ | > * | (offset1:FGRAPH_FRAME_OFFSET) | <- the offset1 is from here > @@ -82,17 +93,24 @@ > * > * If a backtrace is required, and the real return pointer needs to be > * fetched, then it looks at the task's curr_ret_stack offset, if it > - * is greater than zero (reserved, or right before poped), it would mask > + * is greater than zero (reserved, or right before popped), it would mask > * the value by FGRAPH_FRAME_OFFSET_MASK to get the offset of the > * ftrace_ret_stack structure stored on the shadow stack. > */ > > -#define FGRAPH_FRAME_OFFSET_SIZE 10 > -#define FGRAPH_FRAME_OFFSET_MASK GENMASK(FGRAPH_FRAME_OFFSET_SIZE - 1, 0) > +/* > + * The following is for the top word on the stack: > + * > + * FGRAPH_FRAME_OFFSET (0-9) holds the offset delta to the fgraph frame > + * FGRAPH_TYPE (10-11) holds the type of word this is. > + * (RESERVED or BITMAP) > + */ > +#define FGRAPH_FRAME_OFFSET_BITS 10 > +#define FGRAPH_FRAME_OFFSET_MASK GENMASK(FGRAPH_FRAME_OFFSET_BITS - 1, 0) > > -#define FGRAPH_TYPE_SIZE 2 > -#define FGRAPH_TYPE_MASK GENMASK(FGRAPH_TYPE_SIZE - 1, 0) > -#define FGRAPH_TYPE_SHIFT FGRAPH_FRAME_OFFSET_SIZE > +#define FGRAPH_TYPE_BITS 2 > +#define FGRAPH_TYPE_MASK GENMASK(FGRAPH_TYPE_BITS - 1, 0) > +#define FGRAPH_TYPE_SHIFT FGRAPH_FRAME_OFFSET_BITS > > enum { > FGRAPH_TYPE_RESERVED = 0, > @@ -100,31 +118,48 @@ enum { > FGRAPH_TYPE_DATA = 2, > }; > > -#define FGRAPH_INDEX_SIZE 16 > -#define FGRAPH_INDEX_MASK GENMASK(FGRAPH_INDEX_SIZE - 1, 0) > -#define FGRAPH_INDEX_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_SIZE) > +/* > + * For BITMAP type: > + * FGRAPH_INDEX (12-27) bits holding the gops index wanting return callback called > + */ > +#define FGRAPH_INDEX_BITS 16 > +#define FGRAPH_INDEX_MASK GENMASK(FGRAPH_INDEX_BITS - 1, 0) > +#define FGRAPH_INDEX_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS) > > -/* The data size == 0 means 1 word, and 31 (=2^5 - 1) means 32 words. */ > -#define FGRAPH_DATA_SIZE 5 > -#define FGRAPH_DATA_MASK GENMASK(FGRAPH_DATA_SIZE - 1, 0) > -#define FGRAPH_DATA_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_SIZE) > -#define FGRAPH_MAX_DATA_SIZE (sizeof(long) * (1 << FGRAPH_DATA_SIZE)) > +/* > + * For DATA type: > + * FGRAPH_DATA (12-17) bits hold the size of data (in words) > + * FGRAPH_INDEX (18-23) bits hold the index for which gops->idx the data is for > + * > + * Note: > + * data_size == 0 means 1 word, and 31 (=2^5 - 1) means 32 words. > + */ > +#define FGRAPH_DATA_BITS 5 > +#define FGRAPH_DATA_MASK GENMASK(FGRAPH_DATA_BITS - 1, 0) > +#define FGRAPH_DATA_SHIFT (FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS) > +#define FGRAPH_MAX_DATA_SIZE (sizeof(long) * (1 << FGRAPH_DATA_BITS)) > > -#define FGRAPH_DATA_INDEX_SIZE 4 > -#define FGRAPH_DATA_INDEX_MASK GENMASK(FGRAPH_DATA_INDEX_SIZE - 1, 0) > -#define FGRAPH_DATA_INDEX_SHIFT (FGRAPH_DATA_SHIFT + FGRAPH_DATA_SIZE) > +#define FGRAPH_DATA_INDEX_BITS 4 > +#define FGRAPH_DATA_INDEX_MASK GENMASK(FGRAPH_DATA_INDEX_BITS - 1, 0) > +#define FGRAPH_DATA_INDEX_SHIFT (FGRAPH_DATA_SHIFT + FGRAPH_DATA_BITS) > > #define FGRAPH_MAX_INDEX \ > - ((FGRAPH_INDEX_SIZE << FGRAPH_DATA_SIZE) + FGRAPH_RET_INDEX) > + ((FGRAPH_INDEX_SIZE << FGRAPH_DATA_BITS) + FGRAPH_RET_INDEX) > > -#define FGRAPH_ARRAY_SIZE FGRAPH_INDEX_SIZE > +#define FGRAPH_ARRAY_SIZE FGRAPH_INDEX_BITS > > -#define SHADOW_STACK_SIZE (PAGE_SIZE) > -#define SHADOW_STACK_IN_WORD (SHADOW_STACK_SIZE / sizeof(long)) > +/* > + * SHADOW_STACK_SIZE: The size in bytes of the entire shadow stack > + * SHADOW_STACK_OFFSET: The size in long words of the shadow stack > + * SHADOW_STACK_MAX_OFFSET: The max offset of the stack for a new frame to be added > + */ > +#define SHADOW_STACK_SIZE (PAGE_SIZE) > +#define SHADOW_STACK_OFFSET (SHADOW_STACK_SIZE / sizeof(long)) > /* Leave on a buffer at the end */ > #define SHADOW_STACK_MAX_OFFSET \ > - (SHADOW_STACK_IN_WORD - (FGRAPH_FRAME_OFFSET + 1 + FGRAPH_ARRAY_SIZE)) > + (SHADOW_STACK_OFFSET - (FGRAPH_FRAME_OFFSET + 1 + FGRAPH_ARRAY_SIZE)) > > +/* RET_STACK(): Return the frame from a given @offset from task @t */ > #define RET_STACK(t, offset) ((struct ftrace_ret_stack *)(&(t)->ret_stack[offset])) > > /* > @@ -132,12 +167,13 @@ enum { > * ret_stack to store task specific state. > */ > #define SHADOW_STACK_TASK_VARS(ret_stack) \ > - ((unsigned long *)(&(ret_stack)[SHADOW_STACK_IN_WORD - FGRAPH_ARRAY_SIZE])) > + ((unsigned long *)(&(ret_stack)[SHADOW_STACK_OFFSET - FGRAPH_ARRAY_SIZE])) > > DEFINE_STATIC_KEY_FALSE(kill_ftrace_graph); > int ftrace_graph_active; > > static struct fgraph_ops *fgraph_array[FGRAPH_ARRAY_SIZE]; > +static unsigned long fgraph_array_bitmask; > > /* LRU index table for fgraph_array */ > static int fgraph_lru_table[FGRAPH_ARRAY_SIZE]; > @@ -162,6 +198,8 @@ static int fgraph_lru_release_index(int idx) > > fgraph_lru_table[fgraph_lru_last] = idx; > fgraph_lru_last = (fgraph_lru_last + 1) % FGRAPH_ARRAY_SIZE; > + > + clear_bit(idx, &fgraph_array_bitmask); > return 0; > } > > @@ -176,70 +214,77 @@ static int fgraph_lru_alloc_index(void) > > fgraph_lru_table[fgraph_lru_next] = -1; > fgraph_lru_next = (fgraph_lru_next + 1) % FGRAPH_ARRAY_SIZE; > + > + set_bit(idx, &fgraph_array_bitmask); > return idx; > } > > +/* Get the offset to the fgraph frame from a ret_stack value */ > static inline int __get_offset(unsigned long val) > { > return val & FGRAPH_FRAME_OFFSET_MASK; > } > > +/* Get the type of word from a ret_stack value */ > static inline int __get_type(unsigned long val) > { > return (val >> FGRAPH_TYPE_SHIFT) & FGRAPH_TYPE_MASK; > } > > +/* Get the data_index for a DATA type ret_stack word */ > static inline int __get_data_index(unsigned long val) > { > return (val >> FGRAPH_DATA_INDEX_SHIFT) & FGRAPH_DATA_INDEX_MASK; > } > > +/* Get the data_size for a DATA type ret_stack word */ > static inline int __get_data_size(unsigned long val) > { > return ((val >> FGRAPH_DATA_SHIFT) & FGRAPH_DATA_MASK) + 1; > } > > +/* Get the word from the ret_stack at @offset */ > static inline unsigned long get_fgraph_entry(struct task_struct *t, int offset) > { > return t->ret_stack[offset]; > } > > +/* Get the FRAME_OFFSET from the word from the @offset on ret_stack */ > static inline int get_frame_offset(struct task_struct *t, int offset) > { > return __get_offset(t->ret_stack[offset]); > } > > +/* Get FGRAPH_TYPE from the word from the @offset at ret_stack */ > static inline int get_fgraph_type(struct task_struct *t, int offset) > { > return __get_type(t->ret_stack[offset]); > } > > +/* For BITMAP type: get the bitmask from the @offset at ret_stack */ > static inline unsigned long > -get_fgraph_index_bitmap(struct task_struct *t, int offset) > +get_bitmap_bits(struct task_struct *t, int offset) > { > return (t->ret_stack[offset] >> FGRAPH_INDEX_SHIFT) & FGRAPH_INDEX_MASK; > } > > +/* For BITMAP type: set the bits in the bitmap bitmask at @offset on ret_stack */ > static inline void > -set_fgraph_index_bitmap(struct task_struct *t, int offset, unsigned long bitmap) > -{ > - t->ret_stack[offset] = (bitmap << FGRAPH_INDEX_SHIFT) | > - (FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET; > -} > - > -static inline bool is_fgraph_index_set(struct task_struct *t, int offset, int idx) > +set_bitmap_bits(struct task_struct *t, int offset, unsigned long bitmap) > { > - return !!(get_fgraph_index_bitmap(t, offset) & BIT(idx)); > + t->ret_stack[offset] |= (bitmap << FGRAPH_INDEX_SHIFT); > } > > +/* Write the bitmap to the ret_stack at @offset (does index, offset and bitmask) */ > static inline void > -add_fgraph_index_bitmap(struct task_struct *t, int offset, unsigned long bitmap) > +set_bitmap(struct task_struct *t, int offset, unsigned long bitmap) > { > - t->ret_stack[offset] |= (bitmap << FGRAPH_INDEX_SHIFT); > + t->ret_stack[offset] = (bitmap << FGRAPH_INDEX_SHIFT) | > + (FGRAPH_TYPE_BITMAP << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET; > } > > -/* Note: @offset is the offset of FGRAPH_TYPE_DATA entry. */ > -static inline void *get_fgraph_data(struct task_struct *t, int offset) > +/* For DATA type: get the data saved under the ret_stack word at @offset */ > +static inline void *get_data_type_data(struct task_struct *t, int offset) > { > unsigned long val = t->ret_stack[offset]; > > @@ -249,7 +294,8 @@ static inline void *get_fgraph_data(struct task_struct *t, int offset) > return (void *)&t->ret_stack[offset]; > } > > -static inline unsigned long make_fgraph_data(int idx, int size, int offset) > +/* Create the ret_stack word for a DATA type */ > +static inline unsigned long make_data_type_val(int idx, int size, int offset) > { > return (idx << FGRAPH_DATA_INDEX_SHIFT) | > ((size - 1) << FGRAPH_DATA_SHIFT) | > @@ -326,7 +372,7 @@ void *fgraph_reserve_data(int idx, int size_bytes) > if (unlikely(curr_ret_stack >= SHADOW_STACK_MAX_OFFSET)) > return NULL; > > - val = make_fgraph_data(idx, data_size, __get_offset(val) + data_size + 1); > + val = make_data_type_val(idx, data_size, __get_offset(val) + data_size + 1); > > /* Set the last word to be reserved */ > current->ret_stack[curr_ret_stack - 1] = val; > @@ -371,7 +417,7 @@ void *fgraph_retrieve_data(int idx, int *size_bytes) > found: > if (size_bytes) > *size_bytes = __get_data_size(val) * sizeof(long); > - return get_fgraph_data(current, offset); > + return get_data_type_data(current, offset); > } > > /** > @@ -394,6 +440,9 @@ unsigned long *fgraph_get_task_var(struct fgraph_ops *gops) > * @offset: The offset into @t->ret_stack to find the ret_stack entry > * @frame_offset: Where to place the offset into @t->ret_stack of that entry > * > + * Returns a pointer to the previous ret_stack below @offset or NULL > + * when it reaches the bottom of the stack. > + * > * Calling this with: > * > * offset = task->curr_ret_stack; > @@ -493,30 +542,21 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, > if (!current->ret_stack) > return -EBUSY; > > - /* > - * At first, check whether the previous fgraph callback is pushed by > - * the fgraph on the same function entry. > - * But if @func is the self tail-call function, we also need to ensure > - * the ret_stack is not for the previous call by checking whether the > - * bit of @fgraph_idx is set or not. > - */ > - ret_stack = get_ret_stack(current, current->curr_ret_stack, &offset); > - if (ret_stack && ret_stack->func == func && > - get_fgraph_type(current, offset + FGRAPH_FRAME_OFFSET) == FGRAPH_TYPE_BITMAP && > - !is_fgraph_index_set(current, offset + FGRAPH_FRAME_OFFSET, fgraph_idx)) > - return offset + FGRAPH_FRAME_OFFSET; > + BUILD_BUG_ON(SHADOW_STACK_SIZE % sizeof(long)); > > + /* Set val to "reserved" with the delta to the new fgraph frame */ > val = (FGRAPH_TYPE_RESERVED << FGRAPH_TYPE_SHIFT) | FGRAPH_FRAME_OFFSET; > > - BUILD_BUG_ON(SHADOW_STACK_SIZE % sizeof(long)); > - > /* > * We must make sure the ret_stack is tested before we read > * anything else. > */ > smp_rmb(); > > - /* The return trace stack is full */ > + /* > + * Check if there's room on the shadow stack to fit a fraph frame > + * and a bitmap word. > + */ > if (current->curr_ret_stack + FGRAPH_FRAME_OFFSET + 1 >= SHADOW_STACK_MAX_OFFSET) { > atomic_inc(¤t->trace_overrun); > return -EBUSY; > @@ -545,7 +585,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, > * at least a correct offset! > */ > barrier(); > - current->curr_ret_stack = offset + 1; > + WRITE_ONCE(current->curr_ret_stack, offset + 1); > /* > * This next barrier is to ensure that an interrupt coming in > * will not corrupt what we are about to write. > @@ -597,7 +637,8 @@ int function_graph_enter(unsigned long ret, unsigned long func, > if (offset < 0) > goto out; > > - for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) { > + for_each_set_bit(i, &fgraph_array_bitmask, > + sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) { > struct fgraph_ops *gops = fgraph_array[i]; > int save_curr_ret_stack; > > @@ -620,7 +661,7 @@ int function_graph_enter(unsigned long ret, unsigned long func, > * Since this function uses fgraph_idx = 0 as a tail-call checking > * flag, set that bit always. > */ > - set_fgraph_index_bitmap(current, offset, bitmap | BIT(0)); > + set_bitmap(current, offset, bitmap | BIT(0)); > > return 0; > out_ret: > @@ -659,9 +700,9 @@ int function_graph_enter_ops(unsigned long ret, unsigned long func, > save_curr_ret_stack = current->curr_ret_stack; > if (gops->entryfunc(&trace, gops)) { > if (type == FGRAPH_TYPE_RESERVED) > - set_fgraph_index_bitmap(current, offset, BIT(gops->idx)); > + set_bitmap(current, offset, BIT(gops->idx)); > else > - add_fgraph_index_bitmap(current, offset, BIT(gops->idx)); > + set_bitmap_bits(current, offset, BIT(gops->idx)); > return 0; > } else > current->curr_ret_stack = save_curr_ret_stack; > @@ -791,12 +832,11 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > trace.retval = fgraph_ret_regs_return_value(ret_regs); > #endif > > - bitmap = get_fgraph_index_bitmap(current, offset); > - for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) { > + bitmap = get_bitmap_bits(current, offset); > + > + for_each_set_bit(i, &bitmap, sizeof(bitmap) * BITS_PER_BYTE) { > struct fgraph_ops *gops = fgraph_array[i]; > > - if (!(bitmap & BIT(i))) > - continue; > if (gops == &fgraph_stub) > continue; > > @@ -879,9 +919,10 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx, > unsigned long ret, unsigned long *retp) > { > struct ftrace_ret_stack *ret_stack; > + unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler); > int i = task->curr_ret_stack; > > - if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler)) > + if (ret != return_handler) > return ret; > > while (i > 0) { > @@ -891,14 +932,13 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx, > /* > * For the tail-call, there would be 2 or more ftrace_ret_stacks on > * the ret_stack, which records "return_to_handler" as the return > - * address excpt for the last one. > + * address except for the last one. > * But on the real stack, there should be 1 entry because tail-call > * reuses the return address on the stack and jump to the next function. > * Thus we will continue to find real return address. > */ > if (ret_stack->retp == retp && > - ret_stack->ret != > - (unsigned long)dereference_kernel_function_descriptor(return_to_handler)) > + ret_stack->ret != return_handler) > return ret_stack->ret; > } > > @@ -909,10 +949,11 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx, > unsigned long ret, unsigned long *retp) > { > struct ftrace_ret_stack *ret_stack; > + unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler); > int offset = task->curr_ret_stack; > int i; > > - if (ret != (unsigned long)dereference_kernel_function_descriptor(return_to_handler)) > + if (ret != return_handler) > return ret; > > if (!idx) > @@ -921,8 +962,7 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx, > i = *idx; > do { > ret_stack = get_ret_stack(task, offset, &offset); > - if (ret_stack && ret_stack->ret == > - (unsigned long)dereference_kernel_function_descriptor(return_to_handler)) > + if (ret_stack && ret_stack->ret == return_handler) > continue; > i--; > } while (i >= 0 && ret_stack); -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>