From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934035AbcKPT3m (ORCPT ); Wed, 16 Nov 2016 14:29:42 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:50232 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933682AbcKPT3k (ORCPT ); Wed, 16 Nov 2016 14:29:40 -0500 Date: Wed, 16 Nov 2016 11:29:35 -0800 From: "Paul E. McKenney" To: Byungchul Park Cc: josh@joshtriplett.org, rostedt@goodmis.org, mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] rcu: Avoid unnecessary contention of rcu node lock Reply-To: paulmck@linux.vnet.ibm.com References: <1478681833-23397-1-git-send-email-byungchul.park@lge.com> <20161116044931.GD2279@X58A-UD3R> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161116044931.GD2279@X58A-UD3R> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16111619-0012-0000-0000-00001127C7E1 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006089; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000189; SDB=6.00781584; UDB=6.00377043; IPR=6.00559096; BA=6.00004886; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00013348; XFM=3.00000011; UTC=2016-11-16 19:29:37 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16111619-0013-0000-0000-0000473A48C8 Message-Id: <20161116192935.GL3612@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-11-16_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611160290 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 16, 2016 at 01:49:31PM +0900, Byungchul Park wrote: > On Wed, Nov 09, 2016 at 05:57:13PM +0900, Byungchul Park wrote: > > It's unnecessary to try to print stacks of blocked tasks in the case > > that ndetected == 0. Furthermore, calling rcu_print_detail_task_stall() > > causes to acquire rnp locks as many times as the number of leaf nodes > > plus one for root node. It's unnecessary at all in the case. Please accept my apologies for the delay -- the last couple of weeks were quite busy, and I needed to give this the attention that it deserves. > Hello, > > I have two questions. Could you answer them? > > 1. What do you think about this patch? This patch would be a performance optimization if ndetected were often zero at the end of the loop in print_other_cpu_stall(). However, for this to happen, the stall would have to be almost exactly 21 seconds in duration, which seems unlikely and which also proves to be unlikely in actual practice. If there was any performance or readability downside whatsoever for this patch, I would of course need to reject it. However, it appears to be free of any performance degradation and could be said to slightly increase readability. I took the patch and reworked the commit log as shown below. That said, it is quite rare for me to accept a patch with such a low probability of reducing overhead. > 2. Is there a tree where patches about rcu are pulled into, before > being pulled into mainline tree? > For example, tip tree in case of scheduler patches. git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git This is pulled into -tip, as Steven said. Thanx, Paul > It would be appriciated if you answer them. > > Thank you in advance, > Byungchul ------------------------------------------------------------------------ commit 9183b76a762e0e73fd362cf2563f6492ae7fc193 Author: Byungchul Park Date: Wed Nov 9 17:57:13 2016 +0900 rcu: Only dump stalled-tasks stacks if there was a real stall The print_other_cpu_stall() function currently unconditionally invokes rcu_print_detail_task_stall(). This is OK because if there was a stall sufficient to cause print_other_cpu_stall() to be invoked, that stall is very likely to persist through the entire print_other_cpu_stall() execution. However, if the stall did not persist, the variable ndetected will be zero, and that variable is already tested in an "if" statement. Therefore, this commit moves the call to rcu_print_detail_task_stall() under that pre-existing "if" to improve readability, with a very rare reduction in overhead. Signed-off-by: Byungchul Park [ paulmck: Reworked commit log. ] Signed-off-by: Paul E. McKenney diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 2c399db6df6e..b11d00ad1213 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -1504,6 +1504,9 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum) (long)rsp->gpnum, (long)rsp->completed, totqlen); if (ndetected) { rcu_dump_cpu_stacks(rsp); + + /* Complain about tasks blocking the grace period. */ + rcu_print_detail_task_stall(rsp); } else { if (READ_ONCE(rsp->gpnum) != gpnum || READ_ONCE(rsp->completed) == gpnum) { @@ -1520,9 +1523,6 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum) } } - /* Complain about tasks blocking the grace period. */ - rcu_print_detail_task_stall(rsp); - rcu_check_gp_kthread_starvation(rsp); panic_on_rcu_stall();