-ERUNONSUBJECT
-- Steve
On Tue, 17 May 2016 21:31:14 +0530
Soumya PN <soumya....@hpe.com> wrote:
> In ftrace.c inside the function alloc_retstack_tasklist()(which will be
> invoked when function_graph tracing is on) the tasklist_lock is being
> held as reader while iterating through list of threads. Here the lock
> is being held as reader with irqs disabled. The tasklist_lock is never
> write_locked in interrupt context so it is safe to not disable interrupts
> for the duration of read_lock in this block which, can be significant,
> given the block of code iterates through all threads. Hence changing the
> code to call read_lock() and read_unlock() instead of read_lock_irqsave()
> and read_unlock_irqrestore(). Seen the same change is made in
> "kernel/tracepoint.c" and "kernel/sched/core.c"files with commit ids
> 'commit 8063e41d2ffc ("tracing: Change syscall_*regfunc() to check
> PF_KTHREAD and use for_each_process_thread()")'
> 'commit 3472eaa1f12e ("sched: normalize_rt_tasks(): Don't use _irqsave
> for tasklist_lock, use task_rq_lock()")'
>
> Stopped the irqbalance service and bound the interrupts and processes
> being traced to the same cpu core with function_graph tracing on. I/Os
> completed successfully. Verified the trace output and seen the expected
> results.
>
> Signed-off-by: Soumya PN <soumya....@hpe.com>
> ---
> kernel/trace/ftrace.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index b1870fb..a680482 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5713,7 +5713,6 @@ static int alloc_retstack_tasklist(struct
> ftrace_ret_stack **ret_stack_list)
> {
> int i;
> int ret = 0;
> - unsigned long flags;
> int start = 0, end = FTRACE_RETSTACK_ALLOC_SIZE;
> struct task_struct *g, *t;
>
> @@ -5729,7 +5728,7 @@ static int alloc_retstack_tasklist(struct
> ftrace_ret_stack **ret_stack_list)
> }
> }
>
> - read_lock_irqsave(&tasklist_lock, flags);
> + read_lock(&tasklist_lock);
> do_each_thread(g, t) {
> if (start == end) {
> ret = -EAGAIN;
> @@ -5747,7 +5746,7 @@ static int alloc_retstack_tasklist(struct
> ftrace_ret_stack **ret_stack_list)
> } while_each_thread(g, t);
>
> unlock:
> - read_unlock_irqrestore(&tasklist_lock, flags);
> + read_unlock(&tasklist_lock);
> free:
> for (i = start; i < end; i++)
> kfree(ret_stack_list[i]);
