From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752677AbbCWRsp (ORCPT ); Mon, 23 Mar 2015 13:48:45 -0400 Received: from smtprelay0129.hostedemail.com ([216.40.44.129]:49615 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752120AbbCWRsm (ORCPT ); Mon, 23 Mar 2015 13:48:42 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::::,RULES_HIT:41:355:379:541:599:800:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1543:1593:1594:1711:1730:1747:1777:1792:1801:1981:2194:2199:2393:2553:2559:2562:2689:2736:2901:3138:3139:3140:3141:3142:3355:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4605:5007:6119:6261:7875:7903:9149:10004:10226:10400:10848:10967:11026:11232:11658:11914:12043:12438:12517:12519:12663:12740:14093:14097:14254:21080:21088,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: step23_85f4dcac8bd4d X-Filterd-Recvd-Size: 4486 Date: Mon, 23 Mar 2015 13:48:39 -0400 From: Steven Rostedt To: Christoph Lameter Cc: Linus Torvalds , LKML , Ingo Molnar , Andrew Morton , stable , Uwe Kleine-Koenig Subject: Re: [GIT PULL] ring-buffer: Replace this_cpu_*() with __this_cpu_*() Message-ID: <20150323134839.29dfdd08@gandalf.local.home> In-Reply-To: References: <20150319180216.62c9fa4a@gandalf.local.home> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Christoph, Any comment on this? -- Steve On Thu, 19 Mar 2015 15:16:25 -0700 Linus Torvalds wrote: > Ugh. > > I think this is bogus. > > Why? > > On Thu, Mar 19, 2015 at 3:02 PM, Steven Rostedt wrote: > > > > The generic version of this_cpu_read() and write() are: > > > > #define this_cpu_generic_read(pcp) \ > > ({ typeof(pcp) ret__; \ > > preempt_disable(); \ > > ret__ = *this_cpu_ptr(&(pcp)); \ > > preempt_enable(); \ > > ret__; \ > > }) > > > > #define this_cpu_generic_to_op(pcp, val, op) \ > > do { \ > > unsigned long flags; \ > > raw_local_irq_save(flags); \ > > *__this_cpu_ptr(&(pcp)) op val; \ > > raw_local_irq_restore(flags); \ > > } while (0) > > Let's just fix the generic versions of this_cpu_read/write. > > Now, it is true that for the "op" versions of "this_cpu_xyz" we need > to disable preemption or interrupts or something for the generic case > (where "generic case" is weasel-wording for "the architecture is crap > and has horrible problems with any kinds of atomics, even just per-cpu > ones"). > > But that is *not* true for plain read/write. There is no point in > disabling preemption, because in the end, if preemption was enabled, > it happens on a random CPU anyway (and that may be fine - many > heuristics may not care *which* exact CPU it's about), and unlike the > "op" cases, the actual read or write is a single access, so there's no > reason to disable preemption/interrupts to make it "atomic" on that > random CPU. > > If you pair a this_cpu_read with a this_cpu_write and expect them to > go to the same cpu, such a user obviously needs to disable preemption > etc for bigger reasons - but disabling preemption inside the operation > itself does absolutely nothing. > > So is there really any reason to not make those simpler forms just do > something simpler like > > #define this_cpu_generic_read(pcp) \ > READ_ONCE(*this_cpu_ptr(&(pcp))) > > #define this_cpu_generic_read(pcp, val) \ > WRITE_ONCE(*this_cpu_ptr(&(pcp)), val) > > instead? > > Even if we end up being preempted in the middle, do we *care*? It's > going to one or the other CPU. > > The only issue might be CPU hotplug in between (the previous CPU going > away), but I'm not sure even that matterts. > > So I don't think the ring-buffer change is necessarily _wrong_, but if > this is a performance issue, why don't we just fix it up for the > generic case rather than for just one user? > > Or is the hotplug issue a big deal? I thought we already had some > rcu-sched point for the cpu going away, so that even the hotplug case > should be ok. > > Linus