xfs
[Top] [All Lists]

Re: [PATCH v3 03/18] xfs: sparse inode chunks feature helpers and mount

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v3 03/18] xfs: sparse inode chunks feature helpers and mount requirements
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 7 Feb 2015 09:54:52 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1423252385-3063-4-git-send-email-bfoster@xxxxxxxxxx>
References: <1423252385-3063-1-git-send-email-bfoster@xxxxxxxxxx> <1423252385-3063-4-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Feb 06, 2015 at 02:52:50PM -0500, Brian Foster wrote:
> The sparse inode chunks feature uses the helper function to enable the
> allocation of sparse inode chunks. The incompatible feature bit is set
> on disk at mkfs time to prevent mount from unsupported kernels.
> 
> Also, enforce the inode alignment requirements required for sparse inode
> chunks at mount time. When enabled, full inode chunks (and all inode
> record) alignment is increased from cluster size to inode chunk size.
> Sparse inode alignment must match the cluster size of the fs. Both
> superblock alignment fields are set as such by mkfs when sparse inode
> support is enabled.
> 
> Finally, warn that sparse inode chunks is an experimental feature until
> further notice.
> 
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
....
> @@ -182,6 +182,28 @@ xfs_mount_validate_sb(
>                       return -EFSCORRUPTED;
>       }
>  
> +     /*
> +      * Full inode chunks must be aligned to inode chunk size when
> +      * sparse inodes are enabled to support the sparse chunk
> +      * allocation algorithm and prevent overlapping inode records.
> +      */
> +     if (xfs_sb_version_hassparseinodes(sbp)) {
> +             uint32_t        align;
> +
> +             xfs_alert(mp,
> +             "v5 superblock with sparse inode chunk support detected.\n"
> +             "This feature is EXPERIMENTAL. Use at your own risk!");

No need to mention v5 superblocks here - we've already got that in
the initial mount output. Say:

"EXPERIMENTAL sparse inode chunk feature enabled. Use at your own risk!"

> +
> +             align = XFS_INODES_PER_CHUNK * sbp->sb_inodesize
> +                             >> sbp->sb_blocklog;
> +             if (sbp->sb_inoalignmt != align) {
> +                     xfs_warn(mp, "Invalid inode alignment (%u blks). "
> +             "Must match inode chunk size (%u blks) with sparse inodes.",

"Inode block alignment (%u) must match chunk size (%u) for sparse inodes."

That makes both error messages a single line a easy to grep for.

> @@ -738,6 +738,22 @@ xfs_mountfs(
>       }
>  
>       /*
> +      * If enabled, sparse inode chunk alignment is expected to match the
> +      * cluster size. Full inode chunk alignment must match the chunk size,
> +      * but that is checked on sb read verification...
> +      */
> +     if (xfs_sb_version_hassparseinodes(&mp->m_sb) &&
> +         mp->m_sb.sb_spinoalignmt !=
> +         XFS_B_TO_FSBT(mp, mp->m_inode_cluster_size)) {

Indent the second line of the comaprison so it's distinct from the
logic comparison. i.e. at first glance the above looks like three
logic conditions being compared, when in fact it is only two.

        if (xfs_sb_version_hassparseinodes(&mp->m_sb) &&
            mp->m_sb.sb_spinoalignmt !=
                        XFS_B_TO_FSBT(mp, mp->m_inode_cluster_size)) {

This makes it clear at a glance there are only two logic conditions
in the if() statement.

> +             xfs_warn(mp, "Invalid sparse inode chunk alignment (%u blks). "
> +                      "Must match cluster size (%llu blks).",

"Sparse inode chunk block alignment (%u) must match cluster size (%u)."

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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