On Fri, Dec 08, 2023 at 12:16:53PM +1100, NeilBrown wrote: > On Thu, 07 Dec 2023, Donald Buczek wrote: > > > > I'd vote for the subvol-id. I think, with bcachefs a u32 would do, > > but it could be return as u64 for future growth or other filesystems. > > Probably a good choice. Though I'd like to re-emphasise that even doing > this doesn't remove the need to make sure inode numbers are as unique as > they can possibly be made. There are multiple places in /proc (for > example) where a file is identified by fsid and inode number. It isn't > really practical to insert a volume-id in there (though maybe we could > duplicate all those interfaces....). > > Inode numbers would need to be absolutely unique with a volume, and > mostly unique within a filesystem. xor-ing the internal inode number > with a hash of the volume number is one way to achieve this. > > i.e.: while enabling processes that use stat() family function is > clearly import, it does not solve all the problems caused by the > addition of a volume/subvol level between "filesystem" and "inode".
So, I'm hoping we can start pushing the NFS file handle as a standard interface for unique file ID, but since we clearly need something interim/for existing programs: xoring in the subvolume ID sounds like it shouldn't have any real downside, do we want to make that the default? Another possibility: we do have the ability to restrict inode numbers to 32 bits, I've been contemplating using this to report the subvol ID in the top 32 bits. Not my preferred approach, since we currently use the top bits of the inode number for sharding by cpu ID and this would be incompatible with that option, but sounds like it might be worth adding. For plumbing in an option for how to include the subvolume ID in the inode number - we'll add it to fs/bcachefs/opts.h as an OPT_STR() option (i.e. an enum), reserve a few bits in the superblock for it, specify in opts.h when it can be set (at mount, runtime, format time). statx sounds like a no brainer. I don't have the bandwidth to get into that right now, but: inode_inum().subvol is the bcachefs interface for getting it. (Not bi_subvolume in the on disk inode, that's only set on subvolume roots). How does that sound on the bcachefs side, Neil?
