On 10/23/24 9:51 PM, Dave Chinner wrote: > There is one question that needs to be resolved in this patchset: if > we take patch 2 to allow sparse inodes at the end of the AG, why > would we need the change in patch 1? Indeed, at this point I have to > ask why we even need the min/max agbno guidelines to the inode chunk > allocation as we end up allowing any aligned location in the AG to > be used by sparse inodes. i.e. if we take patch 2, then patch 1 is > unnecessary and now we can remove a bunch of code (min/max_agbno > constraints) from the allocator paths... > > I'd prefer that we take the latter path: ignore the first patch. > This results in more flexible behaviour, allows existing filesystems > with this issue to work without needing xfs_repair to fix them, and > we get to remove complexity from the code. > > Thoughts? For some reason I'm struggling to grasp some of the details here, so maybe I can just throw out a "what I think should happen" type response. A concern is that older xfs_repair binaries will continue to see inodes in this region as corrupt, and throw them away, IIUC - even if the kernel is updated to handle them properly. Older xfs_repair could be encountered on rescue CDs/images, maybe even in initramfs environments, by virt hosts managing guest filesystems, etc. So it seems to me that it would be worth it to prevent any new inode allocations in this region going forward, even if we *can* make it work, so that we won't continue to generate what looks like corruption to older userspace. That might not be the most "pure" upstream approach, but as a practical matter I think it might be a better outcome for users and support orgs... even if distros update kernels & userspace together, that does not necessarily prevent older userspace from encountering a filesystem with inodes in this range and trashing them. Thanks, -Eric
