Re: [PATCH 3/5] xfs: rewrite xchk_inode_is_allocated to work properly

Dave Chinner
2023-08-08T15:55:00Z

On Thu, Jul 27, 2023 at 03:31:03PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> 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@birch.djwong.org/
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Lotsa comments, nothing triggered me this time through :)

Reviewed-by: Dave Chinner <dchinner@redhat.com>

--
Dave Chinner
david@fromorbit.com