Re: [RFC-PATCH] xfs: do not update sunit/swidth in the superblock to match those provided during mount

[Dave Chinner <david@fromorbit.com>]

On Wed, Nov 27, 2019 at 06:19:29AM -0800, Christoph Hellwig wrote:
> Can we all take a little step back and think about the implications
> of the original patch from Alex?  Because I think there is very little.
> And updated sunit/swidth is just a little performance optimization,
> and anyone who really cares about changing that after the fact can
> trivially add those to fstab.
> 
> So I think something like his original patch plus a message during
> mount that the new values are not persisted should be perfectly fine.

Well, the original purpose of the mount options was to persist a new
sunit/swidth to the superblock...

Let's ignore the fact that it was a result of a CXFS client mount
bug trashing the existing sunit/swidth values, and instead focus on
the fact we've been telling people for years that you "only need to
set these once after a RAID reshape" and so we have a lot of users
out there expecting it to persist the new values...

I don't think we can just redefine the documented and expected
behaviour of a mount option like this.

With that in mind, the xfs(5) man page explicitly states this:

	The sunit and swidth parameters specified must be compatible
	with the existing filesystem alignment characteristics.  In
	general,  that  means  the  only  valid changes to sunit are
	increasing it by a power-of-2 multiple. Valid swidth values
	are any integer multiple of a valid sunit value.

Note the comment about changes to sunit? What is being done here -
halving the sunit from 64 to 32 blocks is invalid, documented as
invalid, but the kernel does not enforce this. We should fix the
kernel code to enforce the alignment rules that the mount option
is documented to require.

If we want to change the alignment characteristics after mkfs, then
use su=1,sw=1 as the initial values, then the first mount can use
the options to change it to whatever is present after mkfs has run.

Filesystems on storage that has dynamically changeable geometry
probably shouldn't be using fixed physical alignment in the first
place, though...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com