Re: [PATCH] xfs: do not unconditionally enable hasalign feature on V5 filesystems

[Eric Sandeen <sandeen@sandeen.net>]

On 2/15/17 10:13 AM, Eric Sandeen wrote:
>> The root cause of the problem is due to the fact that
>> xfs_sb_version_hasalign() returns true when we are working on a V5
>> filesystem. Due to this args.minalignslop (in xfs_ialloc_ag_alloc())
>> gets the unsigned equivalent of -1 assigned to it. This later causes
>> alloc_len in xfs_alloc_space_available() to have a value of 0. In such a
>> scenario when args.total is also 0, the assert statement
>> "ASSERT(args->maxlen > 0);" fails.
> Hm, the intent of the _haslign() function is to say that V5 must always
> imply the "alignbit" - i.e. we don't want to grow an infinite feature
> matrix, and by the time you get to V5 supers, there are many things which
> cannot be turned on or off, such as this feature.
> 
> So what happens here... xfs_ialloc_ag_alloc does:
> 
> args.minalignslop = xfs_ialloc_cluster_alignment(args.mp) - 1;
> 
> so you're saying that cluster_alignment comes out as 0?
> 
> That function is checking _hasalign:
> 
> static inline int
> xfs_ialloc_cluster_alignment(
>         struct xfs_mount        *mp)
> {
>         if (xfs_sb_version_hasalign(&mp->m_sb) &&
>             mp->m_sb.sb_inoalignmt >=
>                         XFS_B_TO_FSBT(mp, mp->m_inode_cluster_size))
>                 return mp->m_sb.sb_inoalignmt;
>         return 1;
> }
> 
> So are you saying that this function returns 0?  That would imply that
> sb_inoalignmt and m_inode_cluster_size are both zero, yes?  Is this
> what you see?

Sorry, I guess that means XFS_B_TO_FSBT(mp, mp->m_inode_cluster_size)) is
zero; inode cluster size is 8192 in this case I think, and that is in fact
0 filesystem blocks when computed with this macro.

I need to think about this a little bit to convince myself that the inode
alignment bit really /should/ be off for a filesystem of this geometry, vs
changing the macro to recognize the case.

-Eric