Re: [PATCH v2 0/7] xfs: fix [f]inobt magic value verification

[Brian Foster <bfoster@redhat.com>]

On Thu, Jan 31, 2019 at 09:54:04AM -0800, Darrick J. Wong wrote:
> On Thu, Jan 31, 2019 at 10:46:01AM -0500, Brian Foster wrote:
> > Hi all,
> > 
> > Here's v2 of the verifier magic value cleanup. This is generally the
> > same idea as the previous version. The primary difference is that v2
> > first converts verifiers that use cpu order magic comparisons to use
> > on-disk (big endian) order and then stores/compares the verifier magic
> > values in on-disk order. The purpose of this change is to reduce the
> > number of byte swaps required at runtime for verifier magic value
> > checks. Further changes include some cleanups, additional refactoring
> > and the inclusion of Darrick's scrub ->b_ops fix with some modifications
> > from the original version.
> > 
> > This survives fstests on v4 and v5 filesystems on both little and big
> > endian systems without regressions. Thoughts, reviews, flames
> > appreciated.
> 
> One thing I was expecting to see but didn't -- are you going to convert
> the bnobt/cntbt verifiers so that we have tree-specific magic number
> checking?
> 

I originally skipped over that one because the hardcoded checks looked
convoluted. Looking again, we probably should include it simply because
it's a similar shared verifier case as for the inobt vs. finobt. We
might still have to keep around a couple hardcoded magic checks for the
pagf_levels thing, but I'll look into it and tack on a new patch for
xfs_allocbt_buf_ops.

Brian

> --D
> 
> > Brian
> > 
> > v2:
> > - Include djwong's ->b_ops patch w/ modifications.
> > - Added patch to fix up existing cpu endian magic checks, fold in typo
> >   fix.
> > - Replace static inline magic verifier helper with out of line variant,
> >   kill macro.
> > - Store on-disk byte order magics in ->b_ops.
> > - Added patch to refactor common xfs_da3_blkinfo checks.
> > v1: https://marc.info/?l=linux-xfs&m=154878684317178&w=2
> > - Remove endian conversion from helper.
> > - Drop finobt bad magic mitigation patch.
> > - Additional verifier magic fixups.
> > - Add verifier name typo fixup.
> > rfcv2: https://marc.info/?l=linux-xfs&m=154868884311668&w=2
> > - Split off finobt verifier into separate patch, assign it
> >   appropriately.
> > - Created helpers for xfs_buf_ops magic value verification.
> > - Added error mitigation patch for problematic finobt blocks.
> > rfcv1: https://marc.info/?l=linux-xfs&m=154834528212262&w=2
> > 
> > Brian Foster (6):
> >   xfs: always check magic values in on-disk byte order
> >   xfs: create a separate finobt verifier
> >   xfs: distinguish between inobt and finobt magic values
> >   xfs: use verifier magic field in dir2 leaf verifiers
> >   xfs: miscellaneous verifier magic value fixups
> >   xfs: factor xfs_da3_blkinfo verification into common helper
> > 
> > Darrick J. Wong (1):
> >   xfs: set buffer ops when repair probes for btree type
> > 
> >  fs/xfs/libxfs/xfs_ag.c             |   2 +-
> >  fs/xfs/libxfs/xfs_alloc.c          |  12 ++--
> >  fs/xfs/libxfs/xfs_attr_leaf.c      |  21 ++----
> >  fs/xfs/libxfs/xfs_attr_remote.c    |   8 ++-
> >  fs/xfs/libxfs/xfs_bmap_btree.c     |  13 ++--
> >  fs/xfs/libxfs/xfs_da_btree.c       |  50 ++++++++++-----
> >  fs/xfs/libxfs/xfs_da_format.h      |   2 +
> >  fs/xfs/libxfs/xfs_dir2_block.c     |  10 +--
> >  fs/xfs/libxfs/xfs_dir2_data.c      |  12 ++--
> >  fs/xfs/libxfs/xfs_dir2_leaf.c      | 100 ++++++-----------------------
> >  fs/xfs/libxfs/xfs_dir2_node.c      |  11 ++--
> >  fs/xfs/libxfs/xfs_ialloc.c         |   3 +-
> >  fs/xfs/libxfs/xfs_ialloc_btree.c   |  25 +++++---
> >  fs/xfs/libxfs/xfs_inode_buf.c      |   2 +-
> >  fs/xfs/libxfs/xfs_refcount_btree.c |   3 +-
> >  fs/xfs/libxfs/xfs_rmap_btree.c     |   3 +-
> >  fs/xfs/libxfs/xfs_sb.c             |   5 +-
> >  fs/xfs/libxfs/xfs_shared.h         |   1 +
> >  fs/xfs/libxfs/xfs_symlink_remote.c |   3 +-
> >  fs/xfs/scrub/agheader_repair.c     |   2 +-
> >  fs/xfs/scrub/repair.c              |  11 +++-
> >  fs/xfs/xfs_buf.c                   |  41 ++++++++++--
> >  fs/xfs/xfs_buf.h                   |   4 +-
> >  fs/xfs/xfs_log_recover.c           |   6 +-
> >  fs/xfs/xfs_trans_buf.c             |   2 +-
> >  25 files changed, 183 insertions(+), 169 deletions(-)
> > 
> > -- 
> > 2.17.2
> >