From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752825AbdJ0WUt (ORCPT ); Fri, 27 Oct 2017 18:20:49 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:60692 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751659AbdJ0WUo (ORCPT ); Fri, 27 Oct 2017 18:20:44 -0400 Date: Fri, 27 Oct 2017 15:20:39 -0700 From: "Paul E. McKenney" To: Neeraj Upadhyay Cc: josh@joshtriplett.org, rostedt@goodmis.org, mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com, LKML Subject: Re: Query regarding srcu_funnel_exp_start() Reply-To: paulmck@linux.vnet.ibm.com References: <481c3c8d-2385-2e38-dfe6-0cb9b6855b8a@codeaurora.org> <20171027122601.GJ3659@linux.vnet.ibm.com> <946852c9-88cb-7363-e74d-82a1efbd3be1@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <946852c9-88cb-7363-e74d-82a1efbd3be1@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17102722-0052-0000-0000-00000278CB9B X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007964; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000239; SDB=6.00937398; UDB=6.00472463; IPR=6.00717661; BA=6.00005660; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017751; XFM=3.00000015; UTC=2017-10-27 22:20:41 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17102722-0053-0000-0000-00005273C68B Message-Id: <20171027222039.GP3659@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-10-27_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1710270287 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 27, 2017 at 10:15:04PM +0530, Neeraj Upadhyay wrote: > On 10/27/2017 05:56 PM, Paul E. McKenney wrote: > >On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote: > >>Hi, > >> > >>One query regarding srcu_funnel_exp_start() function in > >>kernel/rcu/srcutree.c. > >> > >>static void srcu_funnel_exp_start(struct srcu_struct *sp, struct > >>srcu_node *snp, > >> unsigned long s) > >>{ > >> > >> if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s)) > >> sp->srcu_gp_seq_needed_exp = s; > >> > >>} > >> > >>Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >= > >>'s'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both? > > > >Let's suppose that it is incorrect as currently written. Can you > >construct a test case demonstrating a failure of some sort, then provide > >a fix? > > Will check this. Might take some time to build a test case. Fair enough! I suggest checking to see if kernel/rcu/rcuperf.c can do what you need for this test. (Might not with a single test, but perhaps a before-and-after comparison. Or maybe you really do need to add some test code somewhere.) > >To start with, if it is currently incorrect, what would be the nature > >of the failure? > > > > Thanx, Paul > > > > Hi Paul, > > I see below scenario, where new gp won't be expedited. Please correct > me if I am missing something here. > > 1. CPU0 calls synchronize_srcu_expedited() > > synchronize_srcu_expedited() > __synchronize_srcu() > __call_srcu() > s = rcu_seq_snap(&sp->srcu_gp_seq); // lets say > srcu_gp_seq = 0; > // s = 0x100 Looks like you have one hex digit and then two binary digits, but why not? (RCU_SEQ_STATE_MASK is 3 rather than 0xff.) > sdp->srcu_gp_seq_needed = s // 0x100 > needgp = true > sdp->srcu_gp_seq_needed_exp = s // 0x100 > srcu_funnel_gp_start() > sp->srcu_gp_seq_needed_exp = s; > srcu_gp_start(sp); > rcu_seq_start(&sp->srcu_gp_seq); > > 2. CPU1 calls normal synchronize_srcu() > > synchronize_srcu() > __synchronize_srcu(sp, true) > __call_srcu() > s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1 > // s= 0x200 > sdp->srcu_gp_seq_needed = s; // 0x200 > srcu_funnel_gp_start() > smp_store_release(&sp->srcu_gp_seq_needed, s); // 0x200 > > 3. CPU3 calls synchronize_srcu_expedited() > > synchronize_srcu_expedited() > __synchronize_srcu() > __call_srcu() > s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1 > // s = 0x200 > sdp->srcu_gp_seq_needed_exp = s // 0x200 > srcu_funnel_exp_start(sp, sdp->mynode, s); > // sp->srcu_gp_seq_needed_exp = 0x100 > // s = 0x200 ; sp->srcu_gp_seq_needed_exp is not updated > if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s)) > sp->srcu_gp_seq_needed_exp = s; Seems plausible, but you should be able to show the difference in grace-period duration with a test. While you are in srcu_funnel_exp_start(), should it be rechecking rcu_seq_done(&sp->srcu_gp_seq, s) as well as the current ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s) under the lock? Why or why not? Thanx, Paul