[PATCH v2 00/31] ocfs2: split dinode validation into shared predicates

[Joseph Qi <joseph.qi@linux.alibaba.com>]

ocfs2_validate_inode_block() is a flat sequence of about twenty open-coded
dinode checks.  The filecheck variant duplicates eight of them but reports
differently: mlog() plus an OCFS2_FILECHECK_ERR_* code instead of
ocfs2_error(), which can force the filesystem read-only.  The duplicated
conditions had drifted apart, and filecheck silently skipped the rest.

Patch series summary

  01      Bug fix.  Exempt orphaned dinodes from the zero nlink check.
          An open-unlinked inode legitimately has nlink 0 on disk with
          OCFS2_VALID_FL still set, so the check rejects valid inodes.
          Orphan recovery then skips the resulting bad inode, so the
          orphan is never wiped and its space leaks.  Fixed at the root
          first, so the refactor stays behaviour preserving.

  02-21   Pure refactor, one check per commit.  Each condition becomes a
          static bool ocfs2_dinode_has_*() predicate, following the four
          helpers already in inode.c.  Return codes and log messages stay
          at the call sites, so no ocfs2_error() call site moves and the
          two reporting paths remain distinct.

  22      Bug fix.  filecheck fix reported SUCCESS on corruption it cannot
          repair.  Revalidate and return the residual code, snapshotting
          the block so a failed repair is rolled back rather than leaving
          a dirty buffer behind.

  23-31   Wire the remaining predicates into filecheck.  All reuse
          OCFS2_FILECHECK_ERR_INVALIDINO, so no new error code and no
          change to the userspace-visible names in ocfs2_filecheck_errs[].

The inline xattr check is still not shared: ocfs2_validate_inode_xattr()
calls ocfs2_error() through its helpers in xattr.c, so reusing it needs a
reporting flag threaded into that file.  Left for a follow-up.

Testing

linux-next 20261103, in a QEMU guest running the series.  Block device
buffers are flushed and caches dropped before each remount, otherwise a
corruption test can read a stale buffer and appear to pass.

  - Refactor is behaviour preserving: all 27 messages, all return codes
    and all ocfs2_error()/mlog() call sites in the normal validator are
    unchanged, each predicate body was compared against the condition it
    replaced, and filecheck's disassembly is instruction identical apart
    from __LINE__.  The normal validator's disassembly also shows block
    rescheduling, so equivalence there rests on the source comparison
    rather than an instruction by instruction proof.

  - No false positives on 4K blocks with inline-data, refcount, sparse,
    usrquota, grpquota, xattr and indexed-dirs, over seventeen inode types
    plus all system inodes during mount and I/O; and on 512-byte blocks
    over five types.  Forcing i_xattr_inline_size up on 512-byte blocks
    drove the inline maximum negative and gave "id_count 312 exceeds max
    -88", confirming the comparison stayed signed.

  - Seven corrupted dinodes on a nometaecc volume were each detected with
    the expected message, with no escalation and the filesystem still
    read-write.

  - With metaecc on: a repairable field alone gives SUCCESS and is really
    written to disk; an unrepairable field alone, and a repairable plus
    unrepairable pair, both give INVALIDINO with the on-disk block byte
    identical afterwards and a second check reporting INVALIDINO rather
    than FAILED.

  - An orphaned dinode with nlink 0 reports SUCCESS, while the same dinode
    without OCFS2_ORPHANED_FL still reports INVALIDINO, so the exemption
    did not disable the check.

The ocfs2 testsuite and xfstests were not run.

Changes since v1

  - The zero nlink check rejected valid orphaned dinodes.  Instead of
    exempting them only in filecheck, patch 01 fixes the predicate at the
    root.  The check was added by commit 7efb45f9685f ("ocfs2: invalidate
    inode if i_mode is zero after block read"), whose changelog describes
    a check for "(i_mode == 0, i_nlink == 0, non-orphan)" while the code
    has no orphan exemption and ORs the conditions instead of ANDing
    them.  v1 also understated the reachability: it claimed orphan
    recovery skips validation because ocfs2_read_blocks_sync() takes no
    validate callback, but ocfs2_read_locked_inode() calls
    ocfs2_validate_inode_block() explicitly right after it in the
    !can_lock branch.  Recovery therefore rejects every orphan on every
    pass, and ocfs2_orphan_filldir() ignores the resulting iget failure
    ("Skip bad inodes so that recovery can continue"), so the orphan is
    never wiped and its space is leaked, with a "Corrupt state" message
    logged on each mount.

  - Returning an error from the repair callback after mark_buffer_dirty()
    left a dirty non-uptodate buffer in the block device cache.
    ocfs2_read_blocks() clears BH_Uptodate on validate failure and
    ocfs2_read_locked_inode() bails before the exclusive lock upgrade and
    ocfs2_write_block(), so the next OCFS2_BH_IGNORE_CACHE read skips the
    disk read and fails -EIO, and later filecheck requests report FAILED
    instead of the real reason.  Patch 22 now recomputes the ecc,
    revalidates, and only then marks the buffer dirty, restoring a
    snapshot otherwise.  Two details: the snapshot covers the whole block
    because ocfs2_validate_meta_ecc() mutates it in place during a hamming
    fix, and the ecc is recomputed before revalidating so a bad ecc does
    not roll back its own repair.  The mechanism was confirmed by reading
    those two functions; it was not reproduced by running v1.

  - The l_next_free_rec repair copied an unvalidated l_count, spreading
    the corruption to a second field and stamping it with a good ecc.
    Now guarded by a valid l_count.

  - The suballoc slot commit cited ocfs2_validate_group_descriptor(),
    which does not inspect i_suballoc_slot, and called the range predicate
    stricter than suballoc.c when it is identical.  Corrected to
    ocfs2_get_suballoc_slot_bit() and reworded: the pair of predicates is
    stricter, the range test alone is not.

Link to v1: https://lore.kernel.org/r/20261105094926.3203416-1-joseph.qi@linux.alibaba.com

Joseph Qi (31):
  ocfs2: exempt orphaned dinodes from the zero nlink check
  ocfs2: abstract the zero nlink or mode dinode check
  ocfs2: abstract the dinode i_blkno mismatch check
  ocfs2: abstract the dinode OCFS2_VALID_FL check
  ocfs2: abstract the dinode fs_generation mismatch check
  ocfs2: abstract the dinode suballoc slot checks
  ocfs2: abstract the dinode suballoc bit check
  ocfs2: abstract the dinode orphaned slot check
  ocfs2: abstract the dinode DIO orphaned slot check
  ocfs2: abstract the inline data dinode guard and cluster check
  ocfs2: abstract the inline data id_count bound check
  ocfs2: abstract the inline data i_size bound check
  ocfs2: abstract the fast symlink dinode guard and size check
  ocfs2: abstract the fast symlink NUL-termination check
  ocfs2: abstract the chain list dinode guard and count check
  ocfs2: abstract the chain list next free record check
  ocfs2: abstract the chain list bits per cluster check
  ocfs2: abstract the dinode refcount location check
  ocfs2: abstract the zero extent list count check
  ocfs2: abstract the oversized extent list count check
  ocfs2: abstract the extent list next free record check
  ocfs2: report unrepairable corruption from online filecheck fix
  ocfs2: check dinode nlink and mode in online filecheck
  ocfs2: check dinode suballoc slot in online filecheck
  ocfs2: check dinode suballoc bit in online filecheck
  ocfs2: check dinode orphaned slots in online filecheck
  ocfs2: check inline data dinodes in online filecheck
  ocfs2: check fast symlink dinodes in online filecheck
  ocfs2: check chain list dinodes in online filecheck
  ocfs2: check dinode refcount location in online filecheck
  ocfs2: check extent list dinodes in online filecheck

 fs/ocfs2/inode.c | 512 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 453 insertions(+), 59 deletions(-)

-- 
2.39.3