diff mbox series

[4/6] xfs: add a free space extent change reservation

Message ID 20210527045202.1155628-5-david@fromorbit.com (mailing list archive)
State New, archived
Headers show
Series xfs: bunmapi needs updating for deferred freeing | expand

Commit Message

Dave Chinner May 27, 2021, 4:52 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

Lots of the transaction reservation code reserves space for an
extent allocation. It is inconsistently implemented, and many of
them get it wrong. Introduce a new function to calculate the log
space reservation for adding or removing an extent from the free
space btrees.

This function reserves space for logging the AGF, the AGFL and the
free space btrees, avoiding the need to account for them seperately
in every reservation that manipulates free space.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_trans_resv.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

kernel test robot May 27, 2021, 6:38 a.m. UTC | #1 
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.13-rc3 next-20210526]
[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-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: parisc-randconfig-r036-20210527 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
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/79189c83717c1c3653f6870ab803b4f1eb5d6159
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Chinner/xfs-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
        git checkout 79189c83717c1c3653f6870ab803b4f1eb5d6159
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

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

All warnings (new ones prefixed by >>):

>> fs/xfs/libxfs/xfs_trans_resv.c:91:1: warning: no previous prototype for 'xfs_allocfree_extent_res' [-Wmissing-prototypes]
      91 | xfs_allocfree_extent_res(
         | ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/xfs_allocfree_extent_res +91 fs/xfs/libxfs/xfs_trans_resv.c

    81	
    82	/*
    83	 * Log reservation required to add or remove a single extent to the free space
    84	 * btrees.  This requires modifying:
    85	 *
    86	 * the agf header: 1 sector
    87	 * the agfl header: 1 sector
    88	 * the allocation btrees: 2 trees * (max depth - 1) * block size
    89	 */
    90	uint
  > 91	xfs_allocfree_extent_res(
    92		struct xfs_mount *mp)
    93	{
    94		return xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
    95		       xfs_calc_buf_res(xfs_allocfree_log_count(mp, 1),
    96					XFS_FSB_TO_B(mp, 1));
    97	}
    98	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 27, 2021, 6:38 a.m. UTC | #2 
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.13-rc3 next-20210526]
[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-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
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/79189c83717c1c3653f6870ab803b4f1eb5d6159
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Chinner/xfs-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
        git checkout 79189c83717c1c3653f6870ab803b4f1eb5d6159
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

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

All warnings (new ones prefixed by >>):

>> fs/xfs/libxfs/xfs_trans_resv.c:91:1: warning: no previous prototype for 'xfs_allocfree_extent_res' [-Wmissing-prototypes]
      91 | xfs_allocfree_extent_res(
         | ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/xfs_allocfree_extent_res +91 fs/xfs/libxfs/xfs_trans_resv.c

    81	
    82	/*
    83	 * Log reservation required to add or remove a single extent to the free space
    84	 * btrees.  This requires modifying:
    85	 *
    86	 * the agf header: 1 sector
    87	 * the agfl header: 1 sector
    88	 * the allocation btrees: 2 trees * (max depth - 1) * block size
    89	 */
    90	uint
  > 91	xfs_allocfree_extent_res(
    92		struct xfs_mount *mp)
    93	{
    94		return xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
    95		       xfs_calc_buf_res(xfs_allocfree_log_count(mp, 1),
    96					XFS_FSB_TO_B(mp, 1));
    97	}
    98	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 27, 2021, 7:03 a.m. UTC | #3 
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.13-rc3 next-20210526]
[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-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: x86_64-randconfig-s021-20210527 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/79189c83717c1c3653f6870ab803b4f1eb5d6159
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Chinner/xfs-bunmapi-needs-updating-for-deferred-freeing/20210527-125525
        git checkout 79189c83717c1c3653f6870ab803b4f1eb5d6159
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64 

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


sparse warnings: (new ones prefixed by >>)
>> fs/xfs/libxfs/xfs_trans_resv.c:91:1: sparse: sparse: symbol 'xfs_allocfree_extent_res' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Darrick J. Wong June 2, 2021, 9:37 p.m. UTC | #4 
On Thu, May 27, 2021 at 02:52:00PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Lots of the transaction reservation code reserves space for an
> extent allocation. It is inconsistently implemented, and many of
> them get it wrong. Introduce a new function to calculate the log
> space reservation for adding or removing an extent from the free
> space btrees.
> 
> This function reserves space for logging the AGF, the AGFL and the
> free space btrees, avoiding the need to account for them seperately
> in every reservation that manipulates free space.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_trans_resv.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index d1a0848cb52e..6363cacb790f 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -79,6 +79,23 @@ xfs_allocfree_log_count(
>  	return blocks;
>  }
>  
> +/*
> + * Log reservation required to add or remove a single extent to the free space
> + * btrees.  This requires modifying:
> + *
> + * the agf header: 1 sector
> + * the agfl header: 1 sector
> + * the allocation btrees: 2 trees * (max depth - 1) * block size
> + */
> +uint
> +xfs_allocfree_extent_res(
> +	struct xfs_mount *mp)
> +{
> +	return xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
> +	       xfs_calc_buf_res(xfs_allocfree_log_count(mp, 1),
> +				XFS_FSB_TO_B(mp, 1));
> +}
> +

No caller?  I think the next patch should get merged into this one.

--D

>  /*
>   * Logging inodes is really tricksy. They are logged in memory format,
>   * which means that what we write into the log doesn't directly translate into
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index d1a0848cb52e..6363cacb790f 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -79,6 +79,23 @@  xfs_allocfree_log_count(
 	return blocks;
 }
 
+/*
+ * Log reservation required to add or remove a single extent to the free space
+ * btrees.  This requires modifying:
+ *
+ * the agf header: 1 sector
+ * the agfl header: 1 sector
+ * the allocation btrees: 2 trees * (max depth - 1) * block size
+ */
+uint
+xfs_allocfree_extent_res(
+	struct xfs_mount *mp)
+{
+	return xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
+	       xfs_calc_buf_res(xfs_allocfree_log_count(mp, 1),
+				XFS_FSB_TO_B(mp, 1));
+}
+
 /*
  * Logging inodes is really tricksy. They are logged in memory format,
  * which means that what we write into the log doesn't directly translate into