xfs
[Top] [All Lists]

Re: [PATCH 10/18] xfs: set sparse inodes feature bit when a sparse chunk

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 10/18] xfs: set sparse inodes feature bit when a sparse chunk is allocated
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 28 Jul 2014 12:23:00 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140724224628.GS20518@dastard>
References: <1406211788-63206-1-git-send-email-bfoster@xxxxxxxxxx> <1406211788-63206-11-git-send-email-bfoster@xxxxxxxxxx> <20140724224628.GS20518@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
On Fri, Jul 25, 2014 at 08:46:28AM +1000, Dave Chinner wrote:
> On Thu, Jul 24, 2014 at 10:23:00AM -0400, Brian Foster wrote:
> > The sparse chunk inode record format is backwards compatible with old
> > format inobt records as long as full chunks are allocated. The holemask
> > field uses higher order bytes of the freecount. While sparse chunks can
> > be enabled on previously unsupported fs, older kernel drivers cannot
> > parse sparse inode records.
> > 
> > Set the feature incompatible bit once a sparse inode chunk is allocated
> > to prevent older XFS drivers from tripping over the new format.
> > 
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > ---
> >  fs/xfs/libxfs/xfs_ialloc.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> > index 4226b1b..4dd45c2 100644
> > --- a/fs/xfs/libxfs/xfs_ialloc.c
> > +++ b/fs/xfs/libxfs/xfs_ialloc.c
> > @@ -407,6 +407,27 @@ error:
> >     return error;
> >  }
> >  
> > +STATIC void
> > +xfs_sbversion_add_spinodes(
> > +   struct xfs_trans        *tp,
> > +   struct xfs_mount        *mp)
> > +{
> > +   if (xfs_sb_has_incompat_log_feature(&mp->m_sb,
> > +                                       XFS_SB_FEAT_INCOMPAT_SPINODES))
> 
> wrong function. xfs_sb_has_incompat_feature().
> 

Oops, right.

> > +           return;
> > +
> > +   spin_lock(&mp->m_sb_lock);
> > +   if (xfs_sb_has_incompat_log_feature(&mp->m_sb,
> > +                                       XFS_SB_FEAT_INCOMPAT_SPINODES)) {
> > +           spin_unlock(&mp->m_sb_lock);
> > +           return;
> > +   }
> > +
> > +   mp->m_sb.sb_features_incompat |= XFS_SB_FEAT_INCOMPAT_SPINODES;
> > +   spin_unlock(&mp->m_sb_lock);
> > +   xfs_mod_sb(tp, XFS_SB_FEATURES_INCOMPAT);
> > +}
> > +
> >  /*
> >   * Allocate new inodes in the allocation group specified by agbp.
> >   * Return 0 for success, else error code.
> > @@ -631,6 +652,18 @@ xfs_ialloc_ag_alloc(
> >             if (error)
> >                     return error;
> >     }
> > +
> > +   /*
> > +    * Set an incompat feature bit as old drivers can't parse sparse
> > +    * records. Pre-sparse inode chunk drivers will include the
> > +    * holemask in the higher order freecount bits, resulting in a
> > +    * bogus value.
> > +    *
> > +    * XXX: when is this bit removed?
> > +    */
> > +   if (xfs_inobt_issparse(&rec))
> > +           xfs_sbversion_add_spinodes(tp, args.mp);
> 
> I'd like to avoid this dynamic feature bit adding if possible. Do we
> really need it?
> 

Nope. As discussed, I'll move to something purely manual for the next
iteration and we can move forward from there with regard to if/when to
set it by default.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>