On Wed, Jul 15, 2020 at 10:08:33AM -0400, Brian Foster wrote:
> The inode processing code checks and resets invalid parent values on
> physical inodes in phase 3 but waits to update the parent value in
> the in-core tracking until phase 4. There doesn't appear to be any
> specific reason for the latter beyond caution. In reality, the only
> reason this doesn't cause problems is that phase 3 replaces an
> invalid on-disk parent with another invalid value, so the in-core
> parent returned by phase 4 translates to NULLFSINO.
>
> This is subtle and fragile. To eliminate this duplicate processing
> behavior and break the subtle dependency of requiring an invalid
> dummy value in physical directory inodes, update the in-core parent
> tracking structure at the same point in phase 3 that physical inodes
> are updated. Invalid on-disk parent values will still translate to
> NULLFSINO for the in-core tracking to be identified by later phases.
> This ensures that if a valid dummy value is placed in a physical
> inode (such as rootino) with an invalid parent in phase 3, phase 4
> won't mistakenly return the valid dummy value to be incorrectly set
> in the in-core tracking over the NULLFSINO value that represents the
> broken on-disk state.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks fine to me,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> repair/dino_chunks.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
> index 6685a4d2..96ed6a5b 100644
> --- a/repair/dino_chunks.c
> +++ b/repair/dino_chunks.c
> @@ -859,14 +859,7 @@ next_readbuf:
> */
> if (isa_dir) {
> set_inode_isadir(ino_rec, irec_offset);
> - /*
> - * we always set the parent but
> - * we may as well wait until
> - * phase 4 (no inode discovery)
> - * because the parent info will
> - * be solid then.
> - */
> - if (!ino_discovery) {
> + if (ino_discovery) {
> ASSERT(parent != 0);
> set_inode_parent(ino_rec, irec_offset, parent);
> ASSERT(parent ==
> --
> 2.21.3
>
