On Fri 14-05-21 09:17:30, Darrick J. Wong wrote: > On Fri, May 14, 2021 at 09:19:45AM +1000, Dave Chinner wrote: > > We've been down this path before more than a decade ago when the > > powers that be decreed that inode locking order is to be "by > > structure address" rather than inode number, because "inode number > > is not unique across multiple superblocks". > > > > I'm not sure that there is anywhere that locks multiple inodes > > across different superblocks, but here we are again.... > > Hm. Are there situations where one would want to lock multiple > /mappings/ across different superblocks? The remapping code doesn't > allow cross-super operations, so ... pipes and splice, maybe? I don't > remember that code well enough to say for sure. Splice and friends work one file at a time. I.e., first they fill a pipe from the file with ->read_iter, then they flush the pipe to the target file with ->write_iter. So file locking doesn't get coupled there. > I've been operating under the assumption that as long as one takes all > the same class of lock at the same time (e.g. all the IOLOCKs, then all > the MMAPLOCKs, then all the ILOCKs, like reflink does) that the > incongruency in locking order rules within a class shouldn't be a > problem. That's my understanding as well. > > > It might simply be time to convert all > > > three XFS inode locks to use the same ordering rules. > > > > Careful, there lie dragons along that path because of things like > > how the inode cluster buffer operations work - they all assume > > ascending inode number traversal within and across inode cluster > > buffers and hence we do have locking order constraints based on > > inode number... > > Fair enough, I'll leave the ILOCK alone. :) OK, so should I change the order for invalidate_lock or shall we just leave that alone as it is not a practical problem AFAICT. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
