From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754759AbcEBR5r (ORCPT ); Mon, 2 May 2016 13:57:47 -0400 Received: from e18.ny.us.ibm.com ([129.33.205.208]:50617 "EHLO e18.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753818AbcEBR5j (ORCPT ); Mon, 2 May 2016 13:57:39 -0400 X-IBM-Helo: d01dlp03.pok.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org;target-devel@vger.kernel.org Date: Mon, 2 May 2016 10:57:38 -0700 From: "Paul E. McKenney" To: Christoph Hellwig Cc: Muhammad Falak R Wani , "Nicholas A. Bellinger" , target-devel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] target: use RCU_INIT_POINTER() when NULLing. Message-ID: <20160502175738.GA24455@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <1462107122-9562-1-git-send-email-falakreyaz@gmail.com> <20160501170120.GA15010@infradead.org> <20160501195313.GL3686@linux.vnet.ibm.com> <20160502141023.GA1187@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160502141023.GA1187@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16050217-0045-0000-0000-000004153D87 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 02, 2016 at 07:10:23AM -0700, Paul E. McKenney wrote: > On Sun, May 01, 2016 at 12:53:13PM -0700, Paul E. McKenney wrote: > > On Sun, May 01, 2016 at 10:01:20AM -0700, Christoph Hellwig wrote: > > > On Sun, May 01, 2016 at 06:22:01PM +0530, Muhammad Falak R Wani wrote: > > > > It is safe to use RCU_INIT_POINTER() to NULL, instead of > > > > rcu_assign_pointer(). > > > > This results in slightly smaller/faster code. > > > > > > If this is indeed the case, rcu_assign_pointer should simply check > > > for NULL using __builtin_constant_p and do the right thing transparently > > > instead of burdening it on every user. > > > > Last time around, there was a compiler bug that prevented this from > > working correctly. But it could well be time to look at it again. > > How does the following (untested) patch look? > > Pretty bad, actually... > > People use rcu_assign_pointer() for pointers to functions, which gets > interesting compiler warnings for some configurations. Please see below > for attempt #2. Perhaps third time is the charm? Thanx, Paul ------------------------------------------------------------------------ commit 72a616bf7f99b2ef4f211f73c6def7fa884d6ca4 Author: Paul E. McKenney Date: Sun May 1 18:46:54 2016 -0700 rcu: No ordering for rcu_assign_pointer() of NULL This commit does a compile-time check for rcu_assign_pointer() of NULL, and uses WRITE_ONCE() rather than smp_store_release() in that case. Reported-by: Christoph Hellwig Signed-off-by: Paul E. McKenney diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index c61b6b9506e7..9be61e47badc 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -650,7 +650,16 @@ static inline void rcu_preempt_sleep_check(void) * please be careful when making changes to rcu_assign_pointer() and the * other macros that it invokes. */ -#define rcu_assign_pointer(p, v) smp_store_release(&p, RCU_INITIALIZER(v)) +#define rcu_assign_pointer(p, v) \ +({ \ + uintptr_t _r_a_p__v = (uintptr_t)(v); \ + \ + if (__builtin_constant_p(v) && (_r_a_p__v) == (uintptr_t)NULL) \ + WRITE_ONCE((p), (typeof(p))(_r_a_p__v)); \ + else \ + smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \ + _r_a_p__v; \ +}) /** * rcu_access_pointer() - fetch RCU pointer with no dereferencing