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

[Chandan Rajendra <chandan@linux.vnet.ibm.com>]

On Wednesday, February 15, 2017 09:29:35 PM Eric Sandeen wrote:
> On 2/15/17 10:13 AM, Eric Sandeen wrote:
> > 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.
> 
> I'm rethinking this a bit; while my question may have uncovered
> another real bug, I think maybe your patch /is/ ok.
> 
> While the intent /was/ to ensure that some features are not optional
> on V5 filesystems, the mkfs code itself might end up turning off inode
> alignment for sufficiently large filesystem blocks and sufficiently
> small inode sizes.  The reality is, for sufficiently large fs blocks,
> every new inode chunk allocation is aligned, because it is sub-block
> sized.  And so there is nothing uniquely "aligned" about any allocation.
> 
> i.e. on v4 superblocks, the right combination of inode size & fs block
> size will turn off the alignment feature, even if the user did not
> request it.
> 
> So for v5 supers, it does seem odd to report the presence of the
> alignment feature even when the flag is not set in the superblock
> due to the geometry details.
> 
> This goes way back to:
> 
>  04a1e6c5b xfs: add CRC checks to the superblock
> 
> which doesn't really explain why the xfs_sb_version_hasalign got
> changed.
> 
> Anyway; it probably makes sense to do a targeted fix to resolve
> the issue you've run into, but we might want to take a broader
> look at when the alignment feature (and feature flag) gets set
> and reported, to make sure that it is all consistent and as expected.
> 

Right. I will post the new patch (one that uses xfs_icluster_size_fsb()
instead of XFS_B_TO_FSBT()) and then look at the rest of the alignment feature
usages. Thanks for your guidance.

-- 
chandan