Re: [PATCH 05/17] fs: icache lock i_count

[Andrew Morton]

On Fri, 1 Oct 2010 01:55:36 -0400 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:

> On Wed, Sep 29, 2010 at 09:52:29PM -0700, Andrew Morton wrote:
> > On Wed, 29 Sep 2010 22:18:37 +1000 Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > 
> > > -	if (atomic_read(&inode->i_count) != 1)
> > > +	if (inode->i_count != 1)
> > 
> > This really should have been renamed to catch unconverted code.
> > 
> > Such code usually wouldn't compile anyway, but it will if it takes the
> > address of i_count only (for example).
> 
> If people do whacky things they'll lose - there is a reason why C has a
> fairly strict type system after all.  We've changed types of variables
> all the time and we didn't run into problems.

No, we've run into problems *frequently*.  A common case is where we
convert a mutex to a spinlock or vice versa.  If you don't rename the
lock, the code still compiles (with warnings) and crashes horridly at
runtime.

> > And maybe we should access this guy via accessor functions, dunno.
> 
> Seems like complete overkill.

Still wrong.  We do this frequently and we do it in areas where we
believe that the implementation might change in the future.

Had we done it with i_count from day one then this part of the patchset
would be far simpler.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/