On Thu, Jul 27, 2023 at 03:31:03PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > Back in the mists of time[1], I proposed this function to assist the > inode btree scrubbers in checking the inode btree contents against the > allocation state of the inode records. The original version performed a > direct lookup in the inode cache and returned the allocation status if > the cached inode hadn't been reused and wasn't in an intermediate state. > Brian thought it would be better to use the usual iget/irele mechanisms, > so that was changed for the final version. > > Unfortunately, this hasn't aged well -- the IGET_INCORE flag only has > one user and clutters up the regular iget path, which makes it hard to > reason about how it actually works. Worse yet, the inode inactivation > series silently broke it because iget won't return inodes that are > anywhere in the inactivation machinery, even though the caller is > already required to prevent inode allocation and freeing. Inodes in the > inactivation machinery are still allocated, but the current code's > interactions with the iget code prevent us from being able to say that. > > Now that I understand the inode lifecycle better than I did in early > 2017, I now realize that as long as the cached inode hasn't been reused > and isn't actively being reclaimed, it's safe to access the i_mode field > (with the AGI, rcu, and i_flags locks held), and we don't need to worry > about the inode being freed out from under us. > > Therefore, port the original version to modern code structure, which > fixes the brokennes w.r.t. inactivation. In the next patch we'll remove > IGET_INCORE since it's no longer necessary. > > [1] https://lore.kernel.org/linux-xfs/149643868294.23065.8094890990886436794.stgit@xxxxxxxxxxxxxxxx/ > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> Lotsa comments, nothing triggered me this time through :) Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx
