lkml.org 
[lkml]   [2016]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: [PATCH] ftrace: As disabling interrupts is costly and write_lock variant of tasklist_lock is not held from interrupt context it is not necessary to disable interrupts.
Date
Hi Steve,

Could you please explain what this error means?
Is it related to length of subject?
I have run checkpatch.pl on patch and didn't show any error.

Thanks,
Soumya.

-----Original Message-----
From: Steven Rostedt [mailto:rostedt@goodmis.org]
Sent: Thursday, May 19, 2016 1:25 AM
To: N, Soumya P <soumya.p.n@hpe.com>
Cc: mingo@redhat.com; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ftrace: As disabling interrupts is costly and write_lock variant of tasklist_lock is not held from interrupt context it is not necessary to disable interrupts.


-ERUNONSUBJECT

-- Steve


On Tue, 17 May 2016 21:31:14 +0530
Soumya PN <soumya.p.n@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.p.n@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]);

\
 
 \ /
  Last update: 2016-05-19 12:01    [W:0.082 / U:0.796 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site