Re: [RFC, PATCH] xfs: make superblock version checks reflect reality

[Dave Chinner <david@fromorbit.com>]

On Fri, Mar 07, 2014 at 09:55:41AM +1100, Dave Chinner wrote:
> On Thu, Mar 06, 2014 at 10:05:34AM -0800, Christoph Hellwig wrote:
> > On Thu, Mar 06, 2014 at 05:54:50PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > We only support filesystems that have v2 directory support, and than
> > > means all the checking and handling of superblock versions prior to
> > > this support being added is completely unnecessary overhead.
> > > 
> > > Strip out all the version 1-3 support, sanitise the good version
> > > checking to reflect the supported versions, update all the feature
> > > supported functions and clean up all the support bit definitions to
> > > reflect the fact that we no longer care about Irix bootloader flag
> > > regions for v4 feature bits.
> > 
> > Good idea in general, I like it.
.....
> > >  static inline int xfs_sb_version_hasnlink(xfs_sb_t *sbp)
> > >  {
> > > -	return sbp->sb_versionnum == XFS_SB_VERSION_3 ||
> > > -		 (XFS_SB_VERSION_NUM(sbp) >= XFS_SB_VERSION_4 &&
> > > -		  (sbp->sb_versionnum & XFS_SB_VERSION_NLINKBIT));
> > > +	return !!(sbp->sb_versionnum & XFS_SB_VERSION_NLINKBIT);
> > >  }
> > 
> > As we reject filesystems without the nlink bit we should just be able
> > to kill all code protected by xfs_sb_version_hasnlink checks, shouldn't
> > we?
> > Same for addnlink.
> > dirv2 is another candidate.
> 
> Yes, that's true - but I think those are good candidates for
> separate patches that follow up on this one.

Ok, we don't reject filesystems that don't have the NLINK bit set.
Older filesystems that have only v1 inodes won't have that bit
set, and we didn't set NLINK by default in mkfs until late 2007.
Hence we need to keep some form of NLINK support around.

The alternative is to simply set the bit in the superblock if it is
not set, and then just assume everywhere that it is set and we are
using v2 inodes. That will get rid of the hasnlink/addnlink code
needed to modify the superblock when the link count goes above
MAX_NLINK_1, and will result in filesystems always converting v1
inodes to v2 inodes on writeback of dirty inodes. I don't see a
problem with taking this approach, bt maybe I'm missing something?

For the dirv2 function, it's only used for checking at mount time,
so if that is handled bu xfs_sb_good_version() we no longer need
it....

Your thoughts?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs