From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751571AbdFFRBE (ORCPT ); Tue, 6 Jun 2017 13:01:04 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:45817 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751520AbdFFRBC (ORCPT ); Tue, 6 Jun 2017 13:01:02 -0400 Date: Tue, 6 Jun 2017 10:00:45 -0700 From: "Paul E. McKenney" To: Peter Zijlstra Cc: Heiko Carstens , Christian Borntraeger , Paolo Bonzini , linux-kernel@vger.kernel.org, mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com, fweisbec@gmail.com, oleg@redhat.com, kvm@vger.kernel.org, Linus Torvalds , Martin Schwidefsky , linux-s390 Subject: Re: [PATCH RFC tip/core/rcu 1/2] srcu: Allow use of Tiny/Tree SRCU from both process and interrupt context Reply-To: paulmck@linux.vnet.ibm.com References: <20170605220919.GA27820@linux.vnet.ibm.com> <1496700591-30177-1-git-send-email-paulmck@linux.vnet.ibm.com> <20170606105343.ibhzrk6jwhmoja5t@hirez.programming.kicks-ass.net> <20170606152705.GD6681@osiris> <20170606161551.dy5lr6mo6vqujk5d@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170606161551.dy5lr6mo6vqujk5d@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17060617-2213-0000-0000-000001D302ED X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007184; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000212; SDB=6.00871023; UDB=6.00433178; IPR=6.00651016; BA=6.00005402; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00015720; XFM=3.00000015; UTC=2017-06-06 17:00:59 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17060617-2214-0000-0000-00005667126B Message-Id: <20170606170045.GG3721@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-06_12:,, 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-1703280000 definitions=main-1706060292 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 06, 2017 at 06:15:51PM +0200, Peter Zijlstra wrote: > On Tue, Jun 06, 2017 at 05:27:06PM +0200, Heiko Carstens wrote: > > On Tue, Jun 06, 2017 at 04:45:57PM +0200, Christian Borntraeger wrote: > > > > As a side note, I am asking myself, though, why we do need the > > > preempt_disable/enable for the cases where we use the opcodes > > > like lao (atomic load and or to a memory location) and friends. > > > > Because you want the atomic instruction to be executed on the local cpu for > > which you have to per cpu pointer. If you get preempted to a different cpu > > between the ptr__ assignment and lan instruction it might be executed not > > on the local cpu. It's not really a correctness issue. > > As per the previous email, I think it is a correctness issue wrt CPU > hotplug. In the specific case of SRCU, this might actually be OK. We have not yet entered the SRCU read-side critical section, and SRCU grace periods don't interact with CPU hotplug. And the per-CPU variable stick around. So as long as one of the per-CPU counters is incremented properly, it doesn't really matter which one is incremented. But if you overwrite one CPU's counter with the incremented version of some other CPU's counter, yes, that would be very bad indeed. > > #define arch_this_cpu_to_op(pcp, val, op) \ > > { \ > > typedef typeof(pcp) pcp_op_T__; \ > > pcp_op_T__ val__ = (val); \ > > pcp_op_T__ old__, *ptr__; \ > > preempt_disable(); \ > > ptr__ = raw_cpu_ptr(&(pcp)); \ > > asm volatile( \ > > op " %[old__],%[val__],%[ptr__]\n" \ > > : [old__] "=d" (old__), [ptr__] "+Q" (*ptr__) \ > > : [val__] "d" (val__) \ > > : "cc"); \ > > preempt_enable(); \ > > } > > > > #define this_cpu_and_4(pcp, val) arch_this_cpu_to_op(pcp, val, "lan") > > > > However in reality it doesn't matter at all, since all distributions we > > care about have preemption disabled. > > Well, either you support PREEMPT=y or you don't :-) If you do, it needs > to be correct, irrespective of what distro's do with it. > > > So this_cpu_inc() should just generate three instructions: two to calculate > > the percpu pointer and an additional asi for the atomic increment, with > > operand specific serialization. This is supposed to be a lot faster than > > disabling/enabling interrupts around a non-atomic operation. > > So typically we joke about s390 that it has an instruction for this > 'very-complicated-thing', but here you guys do not, what gives? ;-) Even mainframes have finite silicon area? ;-) Thanx, Paul