[Top] [All Lists]

Re: [PATCH 5/5] xfs: increase inode cluster size for v5 filesystems

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/5] xfs: increase inode cluster size for v5 filesystems
From: Ben Myers <bpm@xxxxxxx>
Date: Wed, 6 Nov 2013 15:31:59 -0600
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131105195650.GA6188@dastard>
References: <1383280040-21979-1-git-send-email-david@xxxxxxxxxxxxx> <1383280040-21979-6-git-send-email-david@xxxxxxxxxxxxx> <20131105164307.GD32110@xxxxxxxxxxxxx> <20131105195650.GA6188@dastard>
User-agent: Mutt/1.5.20 (2009-06-14)
Hi Dave,

On Wed, Nov 06, 2013 at 06:56:50AM +1100, Dave Chinner wrote:
> On Tue, Nov 05, 2013 at 08:43:07AM -0800, Christoph Hellwig wrote:
> > > + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > > +         int     new_size = mp->m_inode_cluster_size;
> > > +
> > > +         new_size *= mp->m_sb.sb_inodesize / XFS_DINODE_MIN_SIZE;
> > > +         if (mp->m_sb.sb_inoalignmt >= XFS_B_TO_FSBT(mp, new_size))
> > > +                 mp->m_inode_cluster_size = new_size;
> > > +         xfs_info(mp, "Using inode cluster size of %d bytes",
> > > +                  mp->m_inode_cluster_size);
> > 
> > printing this on every mount seem a bit too verbose.
> I'd like to leave it there until we remove the experimental tag from
> the v5 superblock configuration, as there is no good way of
> determining that someone is using a mkfs patched to enable this
> feature yet...

I don't think I have a problem with bumping up the inode cluster size, but I am
a little concerned about two aspects of this patch:

1) Backward compatability issue with earlier v5 filesystems that don't support
the larger inode cluster.  I know it's experimental, but what do those failures
look like?  This strikes me as being kind of scary.

2) I don't like to overload the inode alignment mkfs option to do this.  I
think it would be better if we explicitly set the inode cluster size at mkfs

Or maybe this one should have an incompatability bit.  Maybe it doesn't need to
be a separate mkfs option.


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