On Sat, 28 Aug 2021, Christoph Hellwig wrote: > On Sat, Aug 28, 2021 at 09:05:08AM +1000, NeilBrown wrote: > > There doesn't seem to be any other option - and this is still an > > improvement over the current behaviour. > > > > Collisions should still be quite a few years away, and there is hope > > that the btrfs developers will take action before then to provide some > > certainty. Not much hope, but some. > > I think that is a very dangerous assumption. Given how every inode > allocation tends to be somewhat predictable I'm also really worried > that this actually opens up an attach vector. Last but not least It doesn't 'open up' anything because it is already possible to cause inode number collisions for NFS mounts of BTRFS. So this patch is a net improvement. I agree that it isn't perfect, but it is the best I have managed to find and it does solve real problems. Can you suggest any way to make it better? > I also very much disagree with any of the impact to common code. > Most importantly the kstat structure, which exist to support the stat > family of system calls and not as a side channel for NFS file handles > (nevermind that it is hidden in a nfs patch and didn't even Cc the > fsdevel list), but also all the impact to the generic nfsd code for > this very broken concept. If you want to support such a scheme in > btrfs as the lesser of evils (which I disagree with), please make > sure it stays self-contained in the btrfs specific file handle > encoding and decoding. Making the change purely in btrfs is simply not possible. There is no way for btrfs to provide nfsd with a different inode number. To move the bulk of the change into btrfs code we would need - at the very least - some way for nfsd to provide the filehandle when requesting stat information. We would also need to provide a reference filehandle when requesting a dentry->filehandle conversion. Cluttering the export_operations like that just for btrfs doesn't seem like the right balance. I agree that cluttering kstat is not ideal, but it was a case of choosing the minimum change for the maximum effect. fsdevel *was* cc:ed on the early discussion of this patch https://lwn.net/ml/linux-fsdevel/162969155423.9892.18322100025025288277@xxxxxxxxxxxxxxxxxxxxx/ I felt we were at a point where I really needed to focus in on the nfsd side of the discussion, with the nfsd developers. As you are probably aware I have already been through several approaches to solve this problem. I'm not against exploring other avenues, but only if they are genuinely likely to provide measurably better results. I'd be very happy to hear your suggestions. NeilBrown
