From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754385AbcKQDmm (ORCPT ); Wed, 16 Nov 2016 22:42:42 -0500 Received: from LGEAMRELO11.lge.com ([156.147.23.51]:38490 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752742AbcKQDml (ORCPT ); Wed, 16 Nov 2016 22:42:41 -0500 X-Original-SENDERIP: 156.147.1.125 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 10.177.222.33 X-Original-MAILFROM: byungchul.park@lge.com Date: Thu, 17 Nov 2016 12:38:39 +0900 From: Byungchul Park To: "Paul E. McKenney" 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 Message-ID: <20161117033839.GE2279@X58A-UD3R> References: <1478681833-23397-1-git-send-email-byungchul.park@lge.com> <20161116044931.GD2279@X58A-UD3R> <20161116192935.GL3612@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161116192935.GL3612@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 16, 2016 at 11:29:35AM -0800, Paul E. McKenney wrote: > 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. Hello paul, Yes, it's true with current code. > > 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. Thank you very much ;) Thanks, Byungchul > > > 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();