On Tue, Apr 09, 2024 at 05:05:28PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > Per some very late review comments, capture the generation numbers of > both inodes involved in a file content exchange operation so that we > don't accidentally target files with have been reallocated. <sigh> That's a really poor commit message, and that's ignoring the fact the change is actually unnecessary. The issue raised during review didn't need code to be added - it was a question about inode lifecycles and interactions with user driven intent chains. Instead of discussing and working through the issue raised to determine if it was a real issue or not, you immediately assumed everything had to change everywhere and started changing code. Then it turned into a hot mess and you started ranting and lecturing people about how you do want critical reviews of this code, and now we have this completely unexplained, unnecessary patch in the series. Yes, I did perform a followup investigation to that was needed to answer the question I had posed during review. The question was whether the intent recovery at the end of replay is subject to inode life-cycle events during the post-intent, pre-done portion of recovery. Fundamentally, intent chains run in a context that holds an inode reference aren't subject to inode life cycle issues and so we don't need the generation number in the intent to identify the inode. I'd largely forgotten all this because I haven't looked at BUIs and intent extent maps for a -long- time and so I forgot all about the inode numbers they encode and the reasons they don't need generation numbers. i.e. because we can't free an inode while there is an open, unresolved intent chain running, there can't be any life cycle issues with inode numbers in the journal. In the case of exchange: - exchange is done with a reference to the inode via open file descriptors. - the ofds cannot be released until the exchange operation returns to userspace. - the last reference to the inode is therefore held until after the entire intent chain is committed to the journal. - therefore, inode freeing can only occur after the exchange returns to userspace and so can only occur in the journal -after- the intent chain is complete in the journal. Therefore: if the intent chain in the journal is not complete we are guaranteed that the inode in the exchange items is live and valid in the filesytem and the intent chain is acting on the current lifecycle instance of the inode. So, yeah, we don't need inode generation numbers in intent items that are acting on an inode, and we probably should document that somewhere so we don't forget about it again... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
