From patchwork Sun Mar 23 21:33:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 332930 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 5E25F2C00CF for ; Mon, 24 Mar 2014 08:35:02 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750963AbaCWVeE (ORCPT ); Sun, 23 Mar 2014 17:34:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:21215 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750798AbaCWVeD (ORCPT ); Sun, 23 Mar 2014 17:34:03 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2NLXgK9026856 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 23 Mar 2014 17:33:42 -0400 Received: from redhat.com (vpn1-5-132.ams2.redhat.com [10.36.5.132]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id s2NLXbu5017941; Sun, 23 Mar 2014 17:33:38 -0400 Date: Sun, 23 Mar 2014 23:33:49 +0200 From: "Michael S. Tsirkin" To: Eric Dumazet Cc: Monam Agarwal , davem@davemloft.net, jasowang@redhat.com, xemul@parallels.com, wuzhy@linux.vnet.ibm.com, therbert@google.com, yamato@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com Subject: Re: [PATCH] drivers/net: Use RCU_INIT_POINTER(x, NULL) in tun.c Message-ID: <20140323213349.GA8832@redhat.com> References: <1395599552-7486-1-git-send-email-monamagarwal123@gmail.com> <20140323194113.GA13633@redhat.com> <1395604457.9117.21.camel@edumazet-glaptop2.roam.corp.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1395604457.9117.21.camel@edumazet-glaptop2.roam.corp.google.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sun, Mar 23, 2014 at 12:54:17PM -0700, Eric Dumazet wrote: > On Sun, 2014-03-23 at 21:41 +0200, Michael S. Tsirkin wrote: > > > The rcu_assign_pointer() ensures that the initialization of a structure > > is carried out before storing a pointer to that structure. > > In the case of the NULL pointer, there is no structure to initialize, > > so we can safely drop smp_wmb in this case. > > > > Signed-off-by: Michael S. Tsirkin > > > > -- > > > > Lightly tested. > > v is evaluated twice here but that should be ok since this > > only happens when v is a constant, so evaluating it should > > have no side effects. > > Paul, what do you think? > > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > > index 72bf3a0..d33c9ec 100644 > > --- a/include/linux/rcupdate.h > > +++ b/include/linux/rcupdate.h > > @@ -587,7 +587,8 @@ static inline void rcu_preempt_sleep_check(void) > > */ > > #define rcu_assign_pointer(p, v) \ > > do { \ > > - smp_wmb(); \ > > + if (!__builtin_constant_p(v) || (v)) \ > > + smp_wmb(); \ > > ACCESS_ONCE(p) = RCU_INITIALIZER(v); \ > > } while (0) > > > > Yeah, I suggest you read d322f45ceed525daa changelog ;) > Oh I see. It does not seem hard to silence that warning though. See below. Alternatively apply these patches everywhere though it does look like too much work for too little gain to me. --> rcu: optimize rcu_assign_pointer with NULL The rcu_assign_pointer() dropped __builtin_constant_p check to avoid a compiler warning, but we can actually work around it without adding code. Signed-off-by: Michael S. Tsirkin --- Untested, too late here, sorry. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 72bf3a0..9111d40 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -585,9 +585,14 @@ 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. */ +/* The convoluted __builtin_constant_p logic is here to prevent + * gcc from emitting a warning when passed a pointer to a variable. + */ #define rcu_assign_pointer(p, v) \ do { \ - smp_wmb(); \ + if (!__builtin_constant_p(v) || \ + (__builtin_constant_p(v) ? (v) : NULL)) \ + smp_wmb(); \ ACCESS_ONCE(p) = RCU_INITIALIZER(v); \ } while (0)