From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753910AbcEBOK2 (ORCPT ); Mon, 2 May 2016 10:10:28 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:49563 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753716AbcEBOKX (ORCPT ); Mon, 2 May 2016 10:10:23 -0400 X-IBM-Helo: d01dlp01.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 07:10:23 -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: <20160502141023.GA1187@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160501195313.GL3686@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: 16050214-0057-0000-0000-00000433EB21 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 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. Thanx, Paul ------------------------------------------------------------------------ commit f55c2ffb4e105fa0450fe495788765dffc4b752e 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..9b8828c5a9c2 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(v))(_r_a_p__v)); \ + else \ + smp_store_release(&p, RCU_INITIALIZER((typeof(v))_r_a_p__v)); \ + _r_a_p__v; \ +}) /** * rcu_access_pointer() - fetch RCU pointer with no dereferencing