From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933014AbcESJ2T (ORCPT ); Thu, 19 May 2016 05:28:19 -0400 Received: from mail-bn1bon0137.outbound.protection.outlook.com ([157.56.111.137]:59952 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932548AbcESJ2O convert rfc822-to-8bit (ORCPT ); Thu, 19 May 2016 05:28:14 -0400 X-Greylist: delayed 1140 seconds by postgrey-1.27 at vger.kernel.org; Thu, 19 May 2016 05:28:14 EDT From: "N, Soumya P" To: Steven Rostedt 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. Thread-Topic: [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. Thread-Index: AQHRsBpgsSb/EMl5gk2arryxuHY0rp+/Hf2AgADY5aA= Date: Thu, 19 May 2016 08:53:17 +0000 Message-ID: References: <1463500874-77480-1-git-send-email-soumya.p.n@hpe.com> <20160518155433.1631bb23@gandalf.local.home> In-Reply-To: <20160518155433.1631bb23@gandalf.local.home> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: goodmis.org; dkim=none (message not signed) header.d=none;goodmis.org; dmarc=none action=none header.from=hpe.com; x-originating-ip: [15.219.195.7] x-ms-office365-filtering-correlation-id: 9004920f-0322-41f0-5dac-08d37fc30534 x-microsoft-exchange-diagnostics: 1;AT5PR84MB0033;5:1SBA0mvmqDP8HDfqlnB91+zTnnt5XZ2zup3b9ibz8I7gMxxThbIW8cxzlaOvIn+oA63I/4C2oRcHA1dFpEe4iilg1AEnlq/ApuqiwaIwGChiEcv6TCD3hmdWnRauIa0oLyHqQWppBEcJlUKe9wiPiQ==;24:vQv01omNbOZGJodMzwaEzP08JfOfmszxuhykeCWiiH609OW4L5za1M82HUMfmt6ggd7j8cWmJBoxJH8aQQcyUFlUogSSf9Op5vneZvp7geE=;7:RENbukqGcfvwFULn1bjJ7P+RILvR4wnVjLSB80NWCquBd2Q4F5vAJvy/jAJciMiWRB3CuEigdFE/3P88GBGbSBUpqREzA/EiNKyiM6xXWUCRFL1En7Y1NZ1AuogQbA/jmji0NFukDfVGM6rjQhEJiAg8XyrrPYXW4+lVMvRLOp958XEJbHwvxkWpzq+RUsp4 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0033; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:AT5PR84MB0033;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0033; x-forefront-prvs: 094700CA91 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(24454002)(13464003)(377454003)(2906002)(9686002)(11100500001)(4326007)(106116001)(87936001)(1220700001)(99286002)(86362001)(92566002)(19580405001)(81166006)(3846002)(5003600100002)(102836003)(5002640100001)(66066001)(19580395003)(2950100001)(8676002)(6116002)(2900100001)(586003)(5008740100001)(189998001)(10400500002)(8936002)(50986999)(33656002)(122556002)(110136002)(76176999)(54356999)(77096005)(5004730100002);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0033;H:AT5PR84MB0036.NAMPRD84.PROD.OUTLOOK.COM;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-originalarrivaltime: 19 May 2016 08:53:17.0165 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0033 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 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 > --- > 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]);