From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Paul E. McKenney" Subject: Re: Using sparse to catch invalid RCU dereferences? Date: Tue, 8 Apr 2008 10:24:36 -0700 Message-ID: <20080408172436.GE8381@linux.vnet.ibm.com> References: <1207605856.12481.35.camel@johannes.berg> <20080408155259.GA8381@linux.vnet.ibm.com> <1207670955.5550.15.camel@johannes.berg> Reply-To: paulmck@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from e36.co.us.ibm.com ([32.97.110.154]:50128 "EHLO e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751928AbYDHRYk (ORCPT ); Tue, 8 Apr 2008 13:24:40 -0400 Content-Disposition: inline In-Reply-To: <1207670955.5550.15.camel@johannes.berg> Sender: linux-sparse-owner@vger.kernel.org List-Id: linux-sparse@vger.kernel.org To: Johannes Berg Cc: Linux Kernel list , linux-sparse On Tue, Apr 08, 2008 at 06:09:15PM +0200, Johannes Berg wrote: > > > So the address_space attribute says what the pointer points to rather > > than where the pointer resides, correct? > > Yeah. It's currently used for __user and __iomem. Using it for rcu > wouldn't be quite the way it was intended, I think, but hey :) ;-) > > It might be. There are a number of places where it is legal to access > > RCU-protected pointers directly, and all of these would need to be > > changed. For example, in the example above, one could do: > > > > foo = NULL; > > Yeah, all of those would lead to sparse warnings. Are we willing to > change all that code? If it found some bugs, I would certainly be in favor! > > I recently tried to modify rcu_assign_pointer() to issue the memory > > memory barrier only when the pointer was non-NULL, but this ended badly. > > Probably because I am not the greatest gcc expert around... We ended > > up having to define an rcu_assign_index() to handle the possibility of > > assigning a zero-value array index, but my attempts to do type-checking > > backfired, and I eventually gave it up. Again, someone a bit more clued > > in to gcc than I am could probably pull it off. > > I don't think I would be that person :) When it comes to gcc extensions and type-casting trickery, I am not all that clued in, to be honest... > > In addition, it is legal to omit rcu_dereference() and rcu_assign_pointer() > > when holding the update-side lock. > > Right. Those too would lead to problems, unless we change that code to > use those (or other) macros. Yep. > > So I very much like this approach in general, but it will require some > > care to implement. I would be very happy to review and comment!!! > > I'll play with it a bit if I get around, was just reviewing some RCU > usage and had the feeling that it should be possible to automate. If it finds a few bugs, it would be worth it! Thanx, Paul