From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753549AbcEWTo1 (ORCPT ); Mon, 23 May 2016 15:44:27 -0400 Received: from mx2.suse.de ([195.135.220.15]:55192 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751963AbcEWTo0 (ORCPT ); Mon, 23 May 2016 15:44:26 -0400 Date: Mon, 23 May 2016 12:44:16 -0700 From: Davidlohr Bueso To: Jason Low Cc: Peter Hurley , jason.low2@hp.com, Waiman Long , Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, Dave Chinner , "Paul E. McKenney" , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE Message-ID: <20160523194416.up365sfbm7f55kbq@linux-uzut.site> References: <1463534783-38814-1-git-send-email-Waiman.Long@hpe.com> <1463534783-38814-3-git-send-email-Waiman.Long@hpe.com> <20160518140436.GA6273@linux-uzut.site> <1463592095.3369.10.camel@j-VirtualBox> <573CB496.4010707@hpe.com> <1463601515.2587.24.camel@j-VirtualBox> <574086FE.6080807@hurleysoftware.com> <1464029181.2479.21.camel@j-VirtualBox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <1464029181.2479.21.camel@j-VirtualBox> User-Agent: Mutt/1.6.1-neo () Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 23 May 2016, Jason Low wrote: >On Sat, 2016-05-21 at 09:04 -0700, Peter Hurley wrote: >> On 05/18/2016 12:58 PM, Jason Low wrote: >> > It should be fine to use the standard READ_ONCE here, even if it's just >> > for documentation, as it's probably not going to cost anything in >> > practice. It would be better to avoid adding any special macros for this >> > which may just add more complexity. >> >> See, I don't understand this line of reasoning at all. >> >> I read this as "it's ok to be non-optimal here where were spinning CPU >> time but not ok to be non-optimal generally elsewhere where it's >> way less important like at init time". > >So I think there is a difference between using it during init time and >using it here where we're spinning. During init time, initializing the >owner field locklessly is normal. No other thread should be concurrently >be writing to the field, since the structure is just getting >initialized, so there are no surprises there. > >Our access of the owner field in this function is special in that we're >using a bit of "lockless magic" to read and write to a field that gets >concurrently accessed without any serialization. Since we're not taking >the wait_lock in a scenario where we'd normally would take a lock, it >would be good to have this documented. > >> And by the way, it's not just "here" but _everywhere_. >> What about reading ->on_cpu locklessly? > >Sure, we could also use READ_ONCE when reading ->on_cpu :) Locking wise we should be covered with ->on_cpu as we're always under rcu_read_lock (barrier, preempt_disable). But I'm not sure if this rule applies throughout the scheduler, however, like it does in, say thread_group_cputime(). cpu_clock_sample() (from posix timers) seems to mix and match being done under rcu. So ultimately I think you're right. Thanks, Davidlohr