Looks good, Reviewed-by: Christoph Hellwig <hch@xxxxxx> On Tue, Feb 23, 2021 at 04:47:47PM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@xxxxxxxxxx> > > We call xfs_dir_ino_validate() for every dir entry in a directory > when doing validity checking of the directory. It calls > xfs_verify_dir_ino() then emits a corruption report if bad or does > error injection if good. It is extremely costly: > > 43.27% [kernel] [k] xfs_dir3_leaf_check_int > 10.28% [kernel] [k] __xfs_dir3_data_check > 6.61% [kernel] [k] xfs_verify_dir_ino > 4.16% [kernel] [k] xfs_errortag_test > 4.00% [kernel] [k] memcpy > 3.48% [kernel] [k] xfs_dir_ino_validate > > 7% of the cpu usage in this directory traversal workload is > xfs_dir_ino_validate() doing absolutely nothing. > > We don't need error injection to simulate a bad inode numbers in the > directory structure because we can do that by fuzzing the structure > on disk. > > And we don't need a corruption report, because the > __xfs_dir3_data_check() will emit one if the inode number is bad. > > So just call xfs_verify_dir_ino() directly here, and get rid of all > this unnecessary overhead: > > 40.30% [kernel] [k] xfs_dir3_leaf_check_int > 10.98% [kernel] [k] __xfs_dir3_data_check > 8.10% [kernel] [k] xfs_verify_dir_ino > 4.42% [kernel] [k] memcpy > 2.22% [kernel] [k] xfs_dir2_data_get_ftype > 1.52% [kernel] [k] do_raw_spin_lock > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_dir2_data.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c > index 375b3edb2ad2..e67fa086f2c1 100644 > --- a/fs/xfs/libxfs/xfs_dir2_data.c > +++ b/fs/xfs/libxfs/xfs_dir2_data.c > @@ -218,7 +218,7 @@ __xfs_dir3_data_check( > */ > if (dep->namelen == 0) > return __this_address; > - if (xfs_dir_ino_validate(mp, be64_to_cpu(dep->inumber))) > + if (!xfs_verify_dir_ino(mp, be64_to_cpu(dep->inumber))) > return __this_address; > if (offset + xfs_dir2_data_entsize(mp, dep->namelen) > end) > return __this_address; > -- > 2.28.0 > ---end quoted text---
