Hi Dave, Thank you for your response. On Sat, Nov 30, 2019 at 10:28 PM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > 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. If I understand your response correctly: - some sunit/swidth changes during mount are legal and some aren't - the legal changes should be persisted in the superblock What about the repair? Even if user performs a legal change, it still breaks the repairability of the file system. For now, we made a local change to not persist sunit/swidth updates in the superblock. Because we must have a working repair, and our kernel (4.14 stable) allows any sunit/swidth changes. We can definitely adhere to the recommended behavior of setting sunit/swidth=1 during mkfs, provided the repair still works after mounting with different sunit/swidth. Thanks, Alex. > > 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@xxxxxxxxxxxxx
