All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 12/36] xfs: Pre-calculate per-AG agino geometry
@ 2021-12-05  4:36 kernel test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2021-12-05  4:36 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 19626 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211203000111.2800982-13-david@fromorbit.com>
References: <20211203000111.2800982-13-david@fromorbit.com>
TO: Dave Chinner <david@fromorbit.com>
TO: linux-xfs(a)vger.kernel.org

Hi Dave,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on xfs-linux/for-next]
[also build test WARNING on v5.16-rc3 next-20211203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dave-Chinner/xfs-more-work-towards-shrinking/20211203-080331
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: i386-randconfig-c001-20211203 (https://download.01.org/0day-ci/archive/20211205/202112051217.mRdxC4Nd-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d30fcadf07ee552f20156ea90be2fdb54cb9cb08)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1970e03e85e3d4f96bdeeb1bc536f025b50737d1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Chinner/xfs-more-work-towards-shrinking/20211203-080331
        git checkout 1970e03e85e3d4f96bdeeb1bc536f025b50737d1
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv6/route.c:4717:35: note: Passing the value 0 via 3rd parameter 'total'
           rt6_upper_bound_set(rt, &weight, total);
                                            ^~~~~
   net/ipv6/route.c:4717:2: note: Calling 'rt6_upper_bound_set'
           rt6_upper_bound_set(rt, &weight, total);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv6/route.c:4704:7: note: Calling 'rt6_is_dead'
           if (!rt6_is_dead(rt)) {
                ^~~~~~~~~~~~~~~
   net/ipv6/route.c:4676:6: note: Left side of '||' is false
           if (rt->fib6_nh->fib_nh_flags & RTNH_F_DEAD ||
               ^
   net/ipv6/route.c:4677:7: note: Left side of '&&' is true
               (rt->fib6_nh->fib_nh_flags & RTNH_F_LINKDOWN &&
                ^
   net/ipv6/route.c:4678:7: note: Calling 'ip6_ignore_linkdown'
                ip6_ignore_linkdown(rt->fib6_nh->fib_nh_dev)))
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/addrconf.h:406:10: note: Assuming field 'ignore_routes_with_linkdown' is 0, which participates in a condition later
           return !!idev->cnf.ignore_routes_with_linkdown;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/addrconf.h:406:2: note: Returning zero, which participates in a condition later
           return !!idev->cnf.ignore_routes_with_linkdown;
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv6/route.c:4678:7: note: Returning from 'ip6_ignore_linkdown'
                ip6_ignore_linkdown(rt->fib6_nh->fib_nh_dev)))
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv6/route.c:4676:2: note: Taking false branch
           if (rt->fib6_nh->fib_nh_flags & RTNH_F_DEAD ||
           ^
   net/ipv6/route.c:4681:2: note: Returning zero, which participates in a condition later
           return false;
           ^~~~~~~~~~~~
   net/ipv6/route.c:4704:7: note: Returning from 'rt6_is_dead'
           if (!rt6_is_dead(rt)) {
                ^~~~~~~~~~~~~~~
   net/ipv6/route.c:4704:2: note: Taking true branch
           if (!rt6_is_dead(rt)) {
           ^
   net/ipv6/route.c:4706:17: note: '__d' initialized to 0
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:102:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           typeof(divisor) __d = divisor;                  \
           ^~~~~~~~~~~~~~~~~~~
   net/ipv6/route.c:4706:17: note: The value 0 is assigned to '__base'
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:104:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           do_div(_tmp, __d);                              \
           ^~~~~~~~~~~~~~~~~
   arch/x86/include/asm/div64.h:25:2: note: expanded from macro 'do_div'
           __base = (base);                                        \
           ^~~~~~~~~~~~~~~
   net/ipv6/route.c:4706:17: note: Left side of '&&' is false
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:104:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           do_div(_tmp, __d);                              \
           ^
   arch/x86/include/asm/div64.h:26:35: note: expanded from macro 'do_div'
           if (__builtin_constant_p(__base) && is_power_of_2(__base)) { \
                                            ^
   net/ipv6/route.c:4706:17: note: Assuming '__high' is not equal to 0
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:104:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           do_div(_tmp, __d);                              \
           ^~~~~~~~~~~~~~~~~
   arch/x86/include/asm/div64.h:32:7: note: expanded from macro 'do_div'
                   if (__high) {                                   \
                       ^~~~~~
   net/ipv6/route.c:4706:17: note: Taking true branch
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:104:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           do_div(_tmp, __d);                              \
           ^
   arch/x86/include/asm/div64.h:32:3: note: expanded from macro 'do_div'
                   if (__high) {                                   \
                   ^
   net/ipv6/route.c:4706:17: note: Division by zero
                   upper_bound = DIV_ROUND_CLOSEST_ULL((u64) (*weight) << 31,
                                 ^
   include/linux/math.h:104:2: note: expanded from macro 'DIV_ROUND_CLOSEST_ULL'
           do_div(_tmp, __d);                              \
           ^~~~~~~~~~~~~~~~~
   arch/x86/include/asm/div64.h:33:21: note: expanded from macro 'do_div'
                           __upper = __high % (__base);            \
                                     ~~~~~~~^~~~~~~~~~
   net/ipv6/route.c:5904:11: warning: Although the value stored to 'count' is used in the enclosing expression, the value is never actually read from 'count' [clang-analyzer-deadcode.DeadStores]
                           return count += w.count;
                                  ^        ~~~~~~~
   net/ipv6/route.c:5904:11: note: Although the value stored to 'count' is used in the enclosing expression, the value is never actually read from 'count'
                           return count += w.count;
                                  ^        ~~~~~~~
   Suppressed 15 warnings (15 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
>> fs/xfs/libxfs/xfs_inode_buf.c:52:2: warning: Value stored to 'agno' is never read [clang-analyzer-deadcode.DeadStores]
           agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_inode_buf.c:52:2: note: Value stored to 'agno' is never read
           agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 9 warnings (8 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   8 warnings generated.
   Suppressed 8 warnings (8 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   8 warnings generated.
   Suppressed 8 warnings (8 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   9 warnings generated.
   fs/xfs/libxfs/xfs_rmap.c:244:2: warning: 4th function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
           xfs_warn(mp,
           ^
   fs/xfs/libxfs/xfs_rmap.c:2677:9: note: Assuming the condition is false
           ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
                  ^
   fs/xfs/libxfs/xfs_format.h:1433:43: note: expanded from macro 'XFS_RMAP_NON_INODE_OWNER'
   #define XFS_RMAP_NON_INODE_OWNER(owner) (!!((owner) & (1ULL << 63)))
                                             ^
   fs/xfs/xfs_linux.h:214:10: note: expanded from macro 'ASSERT'
           (likely(expr) ? (void)0 : asswarn(NULL, #expr, __FILE__, __LINE__))
            ~~~~~~~^~~~~
   include/linux/compiler.h:77:40: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                                               ^
   fs/xfs/libxfs/xfs_rmap.c:2677:41: note: Left side of '||' is true
           ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
                                                  ^
   fs/xfs/libxfs/xfs_rmap.c:2677:2: note: '?' condition is true
           ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
           ^
   fs/xfs/xfs_linux.h:214:3: note: expanded from macro 'ASSERT'
           (likely(expr) ? (void)0 : asswarn(NULL, #expr, __FILE__, __LINE__))
            ^
   include/linux/compiler.h:77:20: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                           ^
   fs/xfs/libxfs/xfs_rmap.c:2682:6: note: Assuming 'error' is 0
           if (error)
               ^~~~~
   fs/xfs/libxfs/xfs_rmap.c:2682:2: note: Taking false branch
           if (error)
           ^
   fs/xfs/libxfs/xfs_rmap.c:2684:6: note: Assuming 'has_record' is not equal to 0
           if (!has_record) {
               ^~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:2684:2: note: Taking false branch
           if (!has_record) {
           ^
   fs/xfs/libxfs/xfs_rmap.c:2689:10: note: Calling 'xfs_rmap_get_rec'
           error = xfs_rmap_get_rec(cur, &irec, &has_record);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:209:6: note: Assuming 'error' is 0
           if (error || !*stat)
               ^~~~~
   fs/xfs/libxfs/xfs_rmap.c:209:6: note: Left side of '||' is false
   fs/xfs/libxfs/xfs_rmap.c:209:15: note: Assuming the condition is false
           if (error || !*stat)
                        ^~~~~~
   fs/xfs/libxfs/xfs_rmap.c:209:2: note: Taking false branch
           if (error || !*stat)
           ^
   fs/xfs/libxfs/xfs_rmap.c:212:6: note: Calling 'xfs_rmap_btrec_to_irec'
           if (xfs_rmap_btrec_to_irec(rec, irec))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:190:9: note: Calling 'xfs_rmap_irec_offset_unpack'
           return xfs_rmap_irec_offset_unpack(be64_to_cpu(rec->rmap.rm_offset),
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.h:70:6: note: Assuming the condition is true
           if (offset & ~(XFS_RMAP_OFF_MASK | XFS_RMAP_OFF_FLAGS))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.h:70:2: note: Taking true branch
           if (offset & ~(XFS_RMAP_OFF_MASK | XFS_RMAP_OFF_FLAGS))
           ^
   fs/xfs/libxfs/xfs_rmap.h:71:3: note: Returning without writing to 'irec->rm_flags'
                   return -EFSCORRUPTED;
                   ^
   fs/xfs/libxfs/xfs_rmap.h:71:3: note: Returning the value -117, which participates in a condition later
                   return -EFSCORRUPTED;
                   ^~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:190:9: note: Returning from 'xfs_rmap_irec_offset_unpack'
           return xfs_rmap_irec_offset_unpack(be64_to_cpu(rec->rmap.rm_offset),
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:190:2: note: Returning without writing to 'irec->rm_flags'
           return xfs_rmap_irec_offset_unpack(be64_to_cpu(rec->rmap.rm_offset),
           ^
   fs/xfs/libxfs/xfs_rmap.c:190:2: note: Returning the value -117, which participates in a condition later
           return xfs_rmap_irec_offset_unpack(be64_to_cpu(rec->rmap.rm_offset),
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:212:6: note: Returning from 'xfs_rmap_btrec_to_irec'
           if (xfs_rmap_btrec_to_irec(rec, irec))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/xfs/libxfs/xfs_rmap.c:212:2: note: Taking true branch
           if (xfs_rmap_btrec_to_irec(rec, irec))
           ^
   fs/xfs/libxfs/xfs_rmap.c:213:3: note: Control jumps to line 241

vim +/agno +52 fs/xfs/libxfs/xfs_inode_buf.c

f0e28280629e0e fs/xfs/libxfs/xfs_inode_buf.c Jeff Layton       2017-12-11  23  
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  24  /*
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  25   * If we are doing readahead on an inode buffer, we might be in log recovery
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  26   * reading an inode allocation buffer that hasn't yet been replayed, and hence
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  27   * has not had the inode cores stamped into it. Hence for readahead, the buffer
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  28   * may be potentially invalid.
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  29   *
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  30   * If the readahead buffer is invalid, we need to mark it with an error and
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  31   * clear the DONE status of the buffer so that a followup read will re-read it
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  32   * from disk. We don't report the error otherwise to avoid warnings during log
06734e3c95a34e fs/xfs/libxfs/xfs_inode_buf.c Keyur Patel       2020-06-29  33   * recovery and we don't get unnecessary panics on debug kernels. We use EIO here
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  34   * because all we want to do is say readahead failed; there is no-one to report
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  35   * the error to, so this will distinguish it from a non-ra verifier failure.
06734e3c95a34e fs/xfs/libxfs/xfs_inode_buf.c Keyur Patel       2020-06-29  36   * Changes to this readahead error behaviour also need to be reflected in
7d6a13f023567d fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  37   * xfs_dquot_buf_readahead_verify().
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  38   */
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  39  static void
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  40  xfs_inode_buf_verify(
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  41  	struct xfs_buf	*bp,
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  42  	bool		readahead)
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  43  {
dbd329f1e44ed4 fs/xfs/libxfs/xfs_inode_buf.c Christoph Hellwig 2019-06-28  44  	struct xfs_mount *mp = bp->b_mount;
6a96c5650568a2 fs/xfs/libxfs/xfs_inode_buf.c Darrick J. Wong   2018-03-23  45  	xfs_agnumber_t	agno;
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  46  	int		i;
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  47  	int		ni;
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  48  
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  49  	/*
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  50  	 * Validate the magic number and version of every inode in the buffer
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  51  	 */
04fcad80cd0687 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2021-08-18 @52  	agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  53  	ni = XFS_BB_TO_FSB(mp, bp->b_length) * mp->m_sb.sb_inopblock;
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  54  	for (i = 0; i < ni; i++) {
de38db7239c4bd fs/xfs/libxfs/xfs_inode_buf.c Christoph Hellwig 2021-10-11  55  		struct xfs_dinode	*dip;
6a96c5650568a2 fs/xfs/libxfs/xfs_inode_buf.c Darrick J. Wong   2018-03-23  56  		xfs_agino_t		unlinked_ino;
de38db7239c4bd fs/xfs/libxfs/xfs_inode_buf.c Christoph Hellwig 2021-10-11  57  		int			di_ok;
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  58  
88ee2df7f25911 fs/xfs/libxfs/xfs_inode_buf.c Christoph Hellwig 2015-06-22  59  		dip = xfs_buf_offset(bp, (i << mp->m_sb.sb_inodelog));
6a96c5650568a2 fs/xfs/libxfs/xfs_inode_buf.c Darrick J. Wong   2018-03-23  60  		unlinked_ino = be32_to_cpu(dip->di_next_unlinked);
15baadf72cedc2 fs/xfs/libxfs/xfs_inode_buf.c Darrick J. Wong   2019-02-16  61  		di_ok = xfs_verify_magic16(bp, dip->di_magic) &&
cf28e17c9186c8 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2021-08-18  62  			xfs_dinode_good_version(mp, dip->di_version) &&
1970e03e85e3d4 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2021-12-03  63  			xfs_verify_agino_or_null(bp->b_pag, unlinked_ino);
1fd7115eda5661 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-12  64  		if (unlikely(XFS_TEST_ERROR(!di_ok, mp,
9e24cfd044853e fs/xfs/libxfs/xfs_inode_buf.c Darrick J. Wong   2017-06-20  65  						XFS_ERRTAG_ITOBP_INOTOBP))) {
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  66  			if (readahead) {
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  67  				bp->b_flags &= ~XBF_DONE;
b79f4a1c68bb99 fs/xfs/libxfs/xfs_inode_buf.c Dave Chinner      2016-01-12  68  				xfs_buf_ioerror(bp, -EIO);
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  69  				return;
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  70  			}
d8914002a03913 fs/xfs/xfs_inode_buf.c        Dave Chinner      2013-08-27  71  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH 12/36] xfs: Pre-calculate per-AG agino geometry
  2021-12-03  0:00 [RFC] [PATCH 00/36] xfs: more work towards shrinking Dave Chinner
@ 2021-12-03  0:00 ` Dave Chinner
  0 siblings, 0 replies; 2+ messages in thread
From: Dave Chinner @ 2021-12-03  0:00 UTC (permalink / raw)
  To: linux-xfs

From: Dave Chinner <dchinner@redhat.com>

There is a lot of overhead in functions like xfs_verify_agino() that
repeatedly calculate the geometry limits of an AG. These can be
pre-calculated as they are static and the verification context has
a per-ag context it can quickly reference.

In the case of xfs_verify_agino(), we now always have a perag
context handy, so we can store the minimum and maximum agino values
in the AG in the perag. This means we don't have to calculate
it on every call and it can be inlined in callers if we move it
to xfs_ag.h.

xfs_verify_agino_or_null() gets the same perag treatment.

xfs_agino_range() is moved to xfs_ag.c as it's not really a type
function, and it's use is largely restricted as the first and last
aginos can be grabbed straight from the perag in most cases.

Note that we leave the original xfs_verify_agino in place in
xfs_types.c as a static function as other callers in that file do
not have per-ag contexts so still need to go the long way. It's been
renamed to xfs_verify_agno_agino() to indicate it takes both an agno
and an agino to differentiate it from new function.

$ size --totals fs/xfs/built-in.a
	   text    data     bss     dec     hex filename
before	1136681  322835     484 1460000  164720 (TOTALS)
after	1136315  322835     484 1459634  1645b2 (TOTALS)

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ag.c        | 39 +++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_ag.h        | 30 +++++++++++++++++++
 fs/xfs/libxfs/xfs_ialloc.c    |  6 ++--
 fs/xfs/libxfs/xfs_inode_buf.c |  3 +-
 fs/xfs/libxfs/xfs_sb.c        |  9 ++++++
 fs/xfs/libxfs/xfs_types.c     | 55 ++++-------------------------------
 fs/xfs/libxfs/xfs_types.h     |  6 ----
 fs/xfs/scrub/agheader.c       |  6 ++--
 fs/xfs/scrub/ialloc.c         |  6 ++--
 fs/xfs/scrub/repair.c         |  9 ++----
 fs/xfs/xfs_inode.c            | 14 ++++-----
 11 files changed, 104 insertions(+), 79 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index d6af12d2510b..730170dc83a9 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -226,6 +226,41 @@ xfs_ag_block_count(
 			mp->m_sb.sb_dblocks);
 }
 
+/* Calculate the first and last possible inode number in an AG. */
+static void
+__xfs_agino_range(
+	struct xfs_mount	*mp,
+	xfs_agblock_t		eoag,
+	xfs_agino_t		*first,
+	xfs_agino_t		*last)
+{
+	xfs_agblock_t		bno;
+
+	/*
+	 * Calculate the first inode, which will be in the first
+	 * cluster-aligned block after the AGFL.
+	 */
+	bno = round_up(XFS_AGFL_BLOCK(mp) + 1, M_IGEO(mp)->cluster_align);
+	*first = XFS_AGB_TO_AGINO(mp, bno);
+
+	/*
+	 * Calculate the last inode, which will be at the end of the
+	 * last (aligned) cluster that can be allocated in the AG.
+	 */
+	bno = round_down(eoag, M_IGEO(mp)->cluster_align);
+	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
+}
+
+void
+xfs_agino_range(
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno,
+	xfs_agino_t		*first,
+	xfs_agino_t		*last)
+{
+	return __xfs_agino_range(mp, xfs_ag_block_count(mp, agno), first, last);
+}
+
 int
 xfs_initialize_perag(
 	struct xfs_mount	*mp,
@@ -303,6 +338,8 @@ xfs_initialize_perag(
 		pag->block_count = __xfs_ag_block_count(mp, index, agcount,
 				dblocks);
 		pag->min_block = XFS_AGFL_BLOCK(mp);
+		__xfs_agino_range(mp, pag->block_count, &pag->agino_min,
+				&pag->agino_max);
 	}
 
 	index = xfs_set_inode_alloc(mp, agcount);
@@ -968,6 +1005,8 @@ xfs_ag_extend_space(
 
 	/* Update perag geometry */
 	pag->block_count = be32_to_cpu(agf->agf_length);
+	__xfs_agino_range(pag->pag_mount, pag->block_count, &pag->agino_min,
+				&pag->agino_max);
 	return 0;
 }
 
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 77640f1409fd..bb9e91bd38e2 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -70,6 +70,8 @@ struct xfs_perag {
 	/* Precalculated geometry info */
 	xfs_agblock_t		block_count;
 	xfs_agblock_t		min_block;
+	xfs_agino_t		agino_min;
+	xfs_agino_t		agino_max;
 
 #ifdef __KERNEL__
 	/* -- kernel only structures below this line -- */
@@ -124,6 +126,8 @@ void xfs_perag_put(struct xfs_perag *pag);
  * Per-ag geometry infomation and validation
  */
 xfs_agblock_t xfs_ag_block_count(struct xfs_mount *mp, xfs_agnumber_t agno);
+void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
+		xfs_agino_t *first, xfs_agino_t *last);
 
 static inline bool
 xfs_verify_agbno(struct xfs_perag *pag, xfs_agblock_t agbno)
@@ -135,6 +139,32 @@ xfs_verify_agbno(struct xfs_perag *pag, xfs_agblock_t agbno)
 	return true;
 }
 
+/*
+ * Verify that an AG inode number pointer neither points outside the AG
+ * nor points at static metadata.
+ */
+static inline bool
+xfs_verify_agino(struct xfs_perag *pag, xfs_agino_t agino)
+{
+	if (agino < pag->agino_min)
+		return false;
+	if (agino > pag->agino_max)
+		return false;
+	return true;
+}
+
+/*
+ * Verify that an AG inode number pointer neither points outside the AG
+ * nor points at static metadata, or is NULLAGINO.
+ */
+static inline bool
+xfs_verify_agino_or_null(struct xfs_perag *pag, xfs_agino_t agino)
+{
+	if (agino == NULLAGINO)
+		return true;
+	return xfs_verify_agino(pag, agino);
+}
+
 /*
  * Perag iteration APIs
  */
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index b4fef0de2bb8..459a352e1790 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -105,7 +105,6 @@ xfs_inobt_get_rec(
 	int				*stat)
 {
 	struct xfs_mount		*mp = cur->bc_mp;
-	xfs_agnumber_t			agno = cur->bc_ag.pag->pag_agno;
 	union xfs_btree_rec		*rec;
 	int				error;
 	uint64_t			realfree;
@@ -116,7 +115,7 @@ xfs_inobt_get_rec(
 
 	xfs_inobt_btrec_to_irec(mp, rec, irec);
 
-	if (!xfs_verify_agino(mp, agno, irec->ir_startino))
+	if (!xfs_verify_agino(cur->bc_ag.pag, irec->ir_startino))
 		goto out_bad_rec;
 	if (irec->ir_count < XFS_INODES_PER_HOLEMASK_BIT ||
 	    irec->ir_count > XFS_INODES_PER_CHUNK)
@@ -137,7 +136,8 @@ xfs_inobt_get_rec(
 out_bad_rec:
 	xfs_warn(mp,
 		"%s Inode BTree record corruption in AG %d detected!",
-		cur->bc_btnum == XFS_BTNUM_INO ? "Used" : "Free", agno);
+		cur->bc_btnum == XFS_BTNUM_INO ? "Used" : "Free",
+		cur->bc_ag.pag->pag_agno);
 	xfs_warn(mp,
 "start inode 0x%x, count 0x%x, free 0x%x freemask 0x%llx, holemask 0x%x",
 		irec->ir_startino, irec->ir_count, irec->ir_freecount,
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index cae9708c8587..9bb1e785245c 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -10,6 +10,7 @@
 #include "xfs_log_format.h"
 #include "xfs_trans_resv.h"
 #include "xfs_mount.h"
+#include "xfs_ag.h"
 #include "xfs_inode.h"
 #include "xfs_errortag.h"
 #include "xfs_error.h"
@@ -59,7 +60,7 @@ xfs_inode_buf_verify(
 		unlinked_ino = be32_to_cpu(dip->di_next_unlinked);
 		di_ok = xfs_verify_magic16(bp, dip->di_magic) &&
 			xfs_dinode_good_version(mp, dip->di_version) &&
-			xfs_verify_agino_or_null(mp, agno, unlinked_ino);
+			xfs_verify_agino_or_null(bp->b_pag, unlinked_ino);
 		if (unlikely(XFS_TEST_ERROR(!di_ok, mp,
 						XFS_ERRTAG_ITOBP_INOTOBP))) {
 			if (readahead) {
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index f4e84aa1d50a..96f06e11ce58 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -196,6 +196,15 @@ xfs_validate_sb_write(
 	    (sbp->sb_fdblocks > sbp->sb_dblocks ||
 	     !xfs_verify_icount(mp, sbp->sb_icount) ||
 	     sbp->sb_ifree > sbp->sb_icount)) {
+	     /*
+		printk("pag blocks %d agblocks %d min_ino %d max_ino %d\n",
+			bp->b_pag->block_count,
+			xfs_ag_block_count(mp, bp->b_pag->pag_agno),
+			bp->b_pag->agino_min, bp->b_pag->agino_max);
+		*/
+		printk("sb dblocks %lld fdblocks %lld icount %lld, ifree %lld\n",
+			sbp->sb_dblocks, sbp->sb_fdblocks, sbp->sb_icount,
+			sbp->sb_ifree);
 		xfs_warn(mp, "SB summary counter sanity check failed");
 		return -EFSCORRUPTED;
 	}
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
index b3c6b0274e95..5c2765934732 100644
--- a/fs/xfs/libxfs/xfs_types.c
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -73,40 +73,12 @@ xfs_verify_fsbext(
 		XFS_FSB_TO_AGNO(mp, fsbno + len - 1);
 }
 
-/* Calculate the first and last possible inode number in an AG. */
-inline void
-xfs_agino_range(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	xfs_agino_t		*first,
-	xfs_agino_t		*last)
-{
-	xfs_agblock_t		bno;
-	xfs_agblock_t		eoag;
-
-	eoag = xfs_ag_block_count(mp, agno);
-
-	/*
-	 * Calculate the first inode, which will be in the first
-	 * cluster-aligned block after the AGFL.
-	 */
-	bno = round_up(XFS_AGFL_BLOCK(mp) + 1, M_IGEO(mp)->cluster_align);
-	*first = XFS_AGB_TO_AGINO(mp, bno);
-
-	/*
-	 * Calculate the last inode, which will be at the end of the
-	 * last (aligned) cluster that can be allocated in the AG.
-	 */
-	bno = round_down(eoag, M_IGEO(mp)->cluster_align);
-	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
-}
-
 /*
  * Verify that an AG inode number pointer neither points outside the AG
  * nor points at static metadata.
  */
-inline bool
-xfs_verify_agino(
+static inline bool
+xfs_verify_agno_agino(
 	struct xfs_mount	*mp,
 	xfs_agnumber_t		agno,
 	xfs_agino_t		agino)
@@ -118,19 +90,6 @@ xfs_verify_agino(
 	return agino >= first && agino <= last;
 }
 
-/*
- * Verify that an AG inode number pointer neither points outside the AG
- * nor points at static metadata, or is NULLAGINO.
- */
-bool
-xfs_verify_agino_or_null(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	xfs_agino_t		agino)
-{
-	return agino == NULLAGINO || xfs_verify_agino(mp, agno, agino);
-}
-
 /*
  * Verify that an FS inode number pointer neither points outside the
  * filesystem nor points at static AG metadata.
@@ -147,7 +106,7 @@ xfs_verify_ino(
 		return false;
 	if (XFS_AGINO_TO_INO(mp, agno, agino) != ino)
 		return false;
-	return xfs_verify_agino(mp, agno, agino);
+	return xfs_verify_agno_agino(mp, agno, agino);
 }
 
 /* Is this an internal inode number? */
@@ -217,12 +176,8 @@ xfs_icount_range(
 	/* root, rtbitmap, rtsum all live in the first chunk */
 	*min = XFS_INODES_PER_CHUNK;
 
-	for_each_perag(mp, agno, pag) {
-		xfs_agino_t	first, last;
-
-		xfs_agino_range(mp, agno, &first, &last);
-		nr_inos += last - first + 1;
-	}
+	for_each_perag(mp, agno, pag)
+		nr_inos += pag->agino_max - pag->agino_min + 1;
 	*max = nr_inos;
 }
 
diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
index b417e8929235..ebf07c99c1ba 100644
--- a/fs/xfs/libxfs/xfs_types.h
+++ b/fs/xfs/libxfs/xfs_types.h
@@ -190,12 +190,6 @@ bool xfs_verify_fsbno(struct xfs_mount *mp, xfs_fsblock_t fsbno);
 bool xfs_verify_fsbext(struct xfs_mount *mp, xfs_fsblock_t fsbno,
 		xfs_fsblock_t len);
 
-void xfs_agino_range(struct xfs_mount *mp, xfs_agnumber_t agno,
-		xfs_agino_t *first, xfs_agino_t *last);
-bool xfs_verify_agino(struct xfs_mount *mp, xfs_agnumber_t agno,
-		xfs_agino_t agino);
-bool xfs_verify_agino_or_null(struct xfs_mount *mp, xfs_agnumber_t agno,
-		xfs_agino_t agino);
 bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino);
 bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino);
 bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino);
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 60d7a7d5ea97..94d000fd97b7 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -902,17 +902,17 @@ xchk_agi(
 
 	/* Check inode pointers */
 	agino = be32_to_cpu(agi->agi_newino);
-	if (!xfs_verify_agino_or_null(mp, agno, agino))
+	if (!xfs_verify_agino_or_null(pag, agino))
 		xchk_block_set_corrupt(sc, sc->sa.agi_bp);
 
 	agino = be32_to_cpu(agi->agi_dirino);
-	if (!xfs_verify_agino_or_null(mp, agno, agino))
+	if (!xfs_verify_agino_or_null(pag, agino))
 		xchk_block_set_corrupt(sc, sc->sa.agi_bp);
 
 	/* Check unlinked inode buckets */
 	for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) {
 		agino = be32_to_cpu(agi->agi_unlinked[i]);
-		if (!xfs_verify_agino_or_null(mp, agno, agino))
+		if (!xfs_verify_agino_or_null(pag, agino))
 			xchk_block_set_corrupt(sc, sc->sa.agi_bp);
 	}
 
diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
index b80a54be8634..e1026e07bf94 100644
--- a/fs/xfs/scrub/ialloc.c
+++ b/fs/xfs/scrub/ialloc.c
@@ -421,10 +421,10 @@ xchk_iallocbt_rec(
 	const union xfs_btree_rec	*rec)
 {
 	struct xfs_mount		*mp = bs->cur->bc_mp;
+	struct xfs_perag		*pag = bs->cur->bc_ag.pag;
 	struct xchk_iallocbt		*iabt = bs->private;
 	struct xfs_inobt_rec_incore	irec;
 	uint64_t			holes;
-	xfs_agnumber_t			agno = bs->cur->bc_ag.pag->pag_agno;
 	xfs_agino_t			agino;
 	xfs_extlen_t			len;
 	int				holecount;
@@ -446,8 +446,8 @@ xchk_iallocbt_rec(
 
 	agino = irec.ir_startino;
 	/* Record has to be properly aligned within the AG. */
-	if (!xfs_verify_agino(mp, agno, agino) ||
-	    !xfs_verify_agino(mp, agno, agino + XFS_INODES_PER_CHUNK - 1)) {
+	if (!xfs_verify_agino(pag, agino) ||
+	    !xfs_verify_agino(pag, agino + XFS_INODES_PER_CHUNK - 1)) {
 		xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 		goto out;
 	}
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 7c519cebb362..d8da7f994669 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -219,16 +219,13 @@ xrep_calc_ag_resblks(
 		usedlen = aglen - freelen;
 		xfs_buf_relse(bp);
 	}
-	xfs_perag_put(pag);
 
 	/* If the icount is impossible, make some worst-case assumptions. */
 	if (icount == NULLAGINO ||
-	    !xfs_verify_agino(mp, sm->sm_agno, icount)) {
-		xfs_agino_t	first, last;
-
-		xfs_agino_range(mp, sm->sm_agno, &first, &last);
-		icount = last - first + 1;
+	    !xfs_verify_agino(pag, icount)) {
+		icount = pag->agino_max - pag->agino_min + 1;
 	}
+	xfs_perag_put(pag);
 
 	/* If the block counts are impossible, make worst-case assumptions. */
 	if (aglen == NULLAGBLOCK ||
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c07043393643..52b296d76d11 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2034,7 +2034,7 @@ xfs_iunlink_update_bucket(
 	xfs_agino_t		old_value;
 	int			offset;
 
-	ASSERT(xfs_verify_agino_or_null(tp->t_mountp, pag->pag_agno, new_agino));
+	ASSERT(xfs_verify_agino_or_null(pag, new_agino));
 
 	old_value = be32_to_cpu(agi->agi_unlinked[bucket_index]);
 	trace_xfs_iunlink_update_bucket(tp->t_mountp, pag->pag_agno, bucket_index,
@@ -2071,7 +2071,7 @@ xfs_iunlink_update_dinode(
 	struct xfs_mount	*mp = tp->t_mountp;
 	int			offset;
 
-	ASSERT(xfs_verify_agino_or_null(mp, pag->pag_agno, next_agino));
+	ASSERT(xfs_verify_agino_or_null(pag, next_agino));
 
 	trace_xfs_iunlink_update_dinode(mp, pag->pag_agno, agino,
 			be32_to_cpu(dip->di_next_unlinked), next_agino);
@@ -2101,7 +2101,7 @@ xfs_iunlink_update_inode(
 	xfs_agino_t		old_value;
 	int			error;
 
-	ASSERT(xfs_verify_agino_or_null(mp, pag->pag_agno, next_agino));
+	ASSERT(xfs_verify_agino_or_null(pag, next_agino));
 
 	error = xfs_imap_to_bp(mp, tp, &ip->i_imap, &ibp);
 	if (error)
@@ -2110,7 +2110,7 @@ xfs_iunlink_update_inode(
 
 	/* Make sure the old pointer isn't garbage. */
 	old_value = be32_to_cpu(dip->di_next_unlinked);
-	if (!xfs_verify_agino_or_null(mp, pag->pag_agno, old_value)) {
+	if (!xfs_verify_agino_or_null(pag, old_value)) {
 		xfs_inode_verifier_error(ip, -EFSCORRUPTED, __func__, dip,
 				sizeof(*dip), __this_address);
 		error = -EFSCORRUPTED;
@@ -2181,7 +2181,7 @@ xfs_iunlink(
 	 */
 	next_agino = be32_to_cpu(agi->agi_unlinked[bucket_index]);
 	if (next_agino == agino ||
-	    !xfs_verify_agino_or_null(mp, pag->pag_agno, next_agino)) {
+	    !xfs_verify_agino_or_null(pag, next_agino)) {
 		xfs_buf_mark_corrupt(agibp);
 		error = -EFSCORRUPTED;
 		goto out;
@@ -2317,7 +2317,7 @@ xfs_iunlink_map_prev(
 		 * Make sure this pointer is valid and isn't an obvious
 		 * infinite loop.
 		 */
-		if (!xfs_verify_agino(mp, pag->pag_agno, unlinked_agino) ||
+		if (!xfs_verify_agino(pag, unlinked_agino) ||
 		    next_agino == unlinked_agino) {
 			XFS_CORRUPTION_ERROR(__func__,
 					XFS_ERRLEVEL_LOW, mp,
@@ -2364,7 +2364,7 @@ xfs_iunlink_remove(
 	 * go on.  Make sure the head pointer isn't garbage.
 	 */
 	head_agino = be32_to_cpu(agi->agi_unlinked[bucket_index]);
-	if (!xfs_verify_agino(mp, pag->pag_agno, head_agino)) {
+	if (!xfs_verify_agino(pag, head_agino)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
 				agi, sizeof(*agi));
 		return -EFSCORRUPTED;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-12-05  4:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-05  4:36 [PATCH 12/36] xfs: Pre-calculate per-AG agino geometry kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2021-12-03  0:00 [RFC] [PATCH 00/36] xfs: more work towards shrinking Dave Chinner
2021-12-03  0:00 ` [PATCH 12/36] xfs: Pre-calculate per-AG agino geometry Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.