Skip to contentSkip to conversation

xfs: cleanup log reservation calculactions

PATCH Superseded Christoph Hellwig 2 participants 2 messages Apr 27, 2010 – Apr 28, 2010 XFS filesystem

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 xfs: cleanup log reservation calculactions

Instead of having small helper functions calling big macros do the calculations for the log reservations directly in the functions. These are mostly 1:1 from the macros execept that the macros kept the quota calculations in their callers.

Signed-off-by Christoph Hellwig <[email protected]>
fs/xfs/xfs_trans.c +407 -43
Index: xfs/fs/xfs/xfs_trans.c
===================================================================
@@ -47,134 +47,498 @@
 
 kmem_zone_t	*xfs_trans_zone;
 
+
+/*
+ * Various log reservation values.
+ *
+ * These are based on the size of the file system block because that is what
+ * most transactions manipulate.  Each adds in an additional 128 bytes per
+ * item logged to try to account for the overhead of the transaction mechanism.
+ *
+ * Note:  Most of the reservations underestimate the number of allocation
+ * groups into which they could free extents in the xfs_bmap_finish() call.
+ * This is because the number in the worst case is quite high and quite
+ * unusual.  In order to fix this we need to change xfs_bmap_finish() to free
+ * extents in only a single AG at a time.  This will require changes to the
+ * EFI code as well, however, so that the EFI for the extents not freed is
+ * logged again in each transaction.  See SGI PV #261917.
+ *
+ * Reservation functions here avoid a huge stack in xfs_trans_init due to
+ * register overflow from temporaries in the calculations.
+ */
+
+
 /*
- * Reservation functions here avoid a huge stack in xfs_trans_init
- * due to register overflow from temporaries in the calculations.
+ * In a write transaction we can allocate a maximum of 2
+ * extents.  This gives:
+ *    the inode getting the new extents: inode size
+ *    the inode's bmap btree: max depth * block size
+ *    the agfs of the ags from which the extents are allocated: 2 * sector
+ *    the superblock free block counter: sector size
+ *    the allocation btrees: 2 exts * 2 trees * (2 * max depth - 1) * block size
+ * And the bmap_finish transaction can free bmap blocks in a join:
+ *    the agfs of the ags containing the blocks: 2 * sector size
+ *    the agfls of the ags containing the blocks: 2 * sector size
+ *    the super block free block counter: sector size
+ *    the allocation btrees: 2 exts * 2 trees * (2 * max depth - 1) * block size
  */
 STATIC uint
-xfs_calc_write_reservation(xfs_mount_t *mp)
+xfs_calc_write_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_WRITE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK)) +
+		 (2 * mp->m_sb.sb_sectsize) +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 2) +
+		 128 * (4 + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) +
+		 	XFS_ALLOCFREE_LOG_COUNT(mp, 2))),
+		((2 * mp->m_sb.sb_sectsize) +
+		 (2 * mp->m_sb.sb_sectsize) +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 2) +
+		 (128 * (5 + XFS_ALLOCFREE_LOG_COUNT(mp, 2))))
+		) + XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * In truncating a file we free up to two extents at once.  We can modify:
+ *    the inode being truncated: inode size
+ *    the inode's bmap btree: (max depth + 1) * block size
+ * And the bmap_finish transaction can free the blocks and bmap blocks:
+ *    the agf for each of the ags: 4 * sector size
+ *    the agfl for each of the ags: 4 * sector size
+ *    the super block to reflect the freed blocks: sector size
+ *    worst case split in allocation btrees per extent assuming 4 extents:
+ *		4 exts * 2 trees * (2 * max depth - 1) * block size
+ *    the inode btree: max depth * blocksize
+ *    the allocation btrees: 2 trees * (max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_itruncate_reservation(xfs_mount_t *mp)
+xfs_calc_itruncate_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ITRUNCATE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1) +
+		 128 * (2 + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK))),
+		(4 * mp->m_sb.sb_sectsize +
+		 4 * mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 4) +
+		 128 * (9 + XFS_ALLOCFREE_LOG_COUNT(mp, 4)) +
+		 (128 * 5) +
+		 XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		 (128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
+		 	 XFS_ALLOCFREE_LOG_COUNT(mp, 1))))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * In renaming a files we can modify:
+ *    the four inodes involved: 4 * inode size
+ *    the two directory btrees: 2 * (max depth + v2) * dir block size
+ *    the two directory bmap btrees: 2 * max depth * block size
+ * And the bmap_finish transaction can free dir and bmap blocks (two sets
+ *	of bmap blocks) giving:
+ *    the agf for the ags in which the blocks live: 3 * sector size
+ *    the agfl for the ags in which the blocks live: 3 * sector size
+ *    the superblock for the free block count: sector size
+ *    the allocation btrees: 3 exts * 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_rename_reservation(xfs_mount_t *mp)
+xfs_calc_rename_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_RENAME_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(4 * mp->m_sb.sb_inodesize +
+		 2 * XFS_DIROP_LOG_RES(mp) +
+		 128 * (4 + 2 * XFS_DIROP_LOG_COUNT(mp))),
+		(3 * mp->m_sb.sb_sectsize +
+		 3 * mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 3) +
+		 (128 * (7 + XFS_ALLOCFREE_LOG_COUNT(mp, 3))))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * For creating a link to an inode:
+ *    the parent directory inode: inode size
+ *    the linked inode: inode size
+ *    the directory btree could split: (max depth + v2) * dir block size
+ *    the directory bmap btree could join or split: (max depth + v2) * blocksize
+ * And the bmap_finish transaction can free some bmap blocks giving:
+ *    the agf for the ag in which the blocks live: sector size
+ *    the agfl for the ag in which the blocks live: sector size
+ *    the superblock for the free block count: sector size
+ *    the allocation btrees: 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_link_reservation(xfs_mount_t *mp)
+xfs_calc_link_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_LINK_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 mp->m_sb.sb_inodesize +
+		 XFS_DIROP_LOG_RES(mp) +
+		 128 * (2 + XFS_DIROP_LOG_COUNT(mp))),
+		(mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		 128 * (3 + XFS_ALLOCFREE_LOG_COUNT(mp, 1)))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * For removing a directory entry we can modify:
+ *    the parent directory inode: inode size
+ *    the removed inode: inode size
+ *    the directory btree could join: (max depth + v2) * dir block size
+ *    the directory bmap btree could join or split: (max depth + v2) * blocksize
+ * And the bmap_finish transaction can free the dir and bmap blocks giving:
+ *    the agf for the ag in which the blocks live: 2 * sector size
+ *    the agfl for the ag in which the blocks live: 2 * sector size
+ *    the superblock for the free block count: sector size
+ *    the allocation btrees: 2 exts * 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_remove_reservation(xfs_mount_t *mp)
+xfs_calc_remove_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_REMOVE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 mp->m_sb.sb_inodesize +
+		 XFS_DIROP_LOG_RES(mp) +
+		 128 * (2 + XFS_DIROP_LOG_COUNT(mp))),
+		(2 * mp->m_sb.sb_sectsize +
+		 2 * mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 2) +
+		 128 * (5 + XFS_ALLOCFREE_LOG_COUNT(mp, 2)))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * For symlink we can modify:
+ *    the parent directory inode: inode size
+ *    the new inode: inode size
+ *    the inode btree entry: 1 block
+ *    the directory btree: (max depth + v2) * dir block size
+ *    the directory inode's bmap btree: (max depth + v2) * block size
+ *    the blocks for the symlink: 1 kB
+ * Or in the first xact we allocate some inodes giving:
+ *    the agi and agf of the ag getting the new inodes: 2 * sectorsize
+ *    the inode blocks allocated: XFS_IALLOC_BLOCKS * blocksize
+ *    the inode btree: max depth * blocksize
+ *    the allocation btrees: 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_symlink_reservation(xfs_mount_t *mp)
+xfs_calc_symlink_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_SYMLINK_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 mp->m_sb.sb_inodesize +
+		 XFS_FSB_TO_B(mp, 1) +
+		 XFS_DIROP_LOG_RES(mp) +
+		 1024 +
+		 128 * (4 + XFS_DIROP_LOG_COUNT(mp))),
+		(2 * mp->m_sb.sb_sectsize +
+		 XFS_FSB_TO_B(mp, XFS_IALLOC_BLOCKS(mp)) +
+		 XFS_FSB_TO_B(mp, mp->m_in_maxlevels) +
+		 XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
+			XFS_ALLOCFREE_LOG_COUNT(mp, 1)))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * For create we can modify:
+ *    the parent directory inode: inode size
+ *    the new inode: inode size
+ *    the inode btree entry: block size
+ *    the superblock for the nlink flag: sector size
+ *    the directory btree: (max depth + v2) * dir block size
+ *    the directory inode's bmap btree: (max depth + v2) * block size
+ * Or in the first xact we allocate some inodes giving:
+ *    the agi and agf of the ag getting the new inodes: 2 * sectorsize
+ *    the superblock for the nlink flag: sector size
+ *    the inode blocks allocated: XFS_IALLOC_BLOCKS * blocksize
+ *    the inode btree: max depth * blocksize
+ *    the allocation btrees: 2 trees * (max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_create_reservation(xfs_mount_t *mp)
+xfs_calc_create_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_CREATE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 mp->m_sb.sb_inodesize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_FSB_TO_B(mp, 1) +
+		 XFS_DIROP_LOG_RES(mp) +
+		 128 * (3 + XFS_DIROP_LOG_COUNT(mp))),
+		(3 * mp->m_sb.sb_sectsize +
+		 XFS_FSB_TO_B(mp, XFS_IALLOC_BLOCKS(mp)) +
+		 XFS_FSB_TO_B(mp, mp->m_in_maxlevels) +
+		 XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
+		 	XFS_ALLOCFREE_LOG_COUNT(mp, 1)))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * Making a new directory is the same as creating a new file.
+ */
 STATIC uint
-xfs_calc_mkdir_reservation(xfs_mount_t *mp)
+xfs_calc_mkdir_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_MKDIR_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return xfs_calc_create_reservation(mp);
 }
 
+/*
+ * In freeing an inode we can modify:
+ *    the inode being freed: inode size
+ *    the super block free inode counter: sector size
+ *    the agi hash list and counters: sector size
+ *    the inode btree entry: block size
+ *    the on disk inode before ours in the agi hash list: inode cluster size
+ *    the inode btree: max depth * blocksize
+ *    the allocation btrees: 2 trees * (max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_ifree_reservation(xfs_mount_t *mp)
+xfs_calc_ifree_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_IFREE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_sectsize +
+		mp->m_sb.sb_sectsize +
+		XFS_FSB_TO_B(mp, 1) +
+		MAX((__uint16_t)XFS_FSB_TO_B(mp, 1),
+		    XFS_INODE_CLUSTER_SIZE(mp)) +
+		(128 * 5) +
+		XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		(128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
+			XFS_ALLOCFREE_LOG_COUNT(mp, 1))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * When only changing the inode we log the inode and possibly the superblock
+ * We also add a bit of slop for the transaction stuff.
+ */
 STATIC uint
-xfs_calc_ichange_reservation(xfs_mount_t *mp)
+xfs_calc_ichange_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ICHANGE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_sectsize +
+		512 +
+		XFS_DQUOT_LOGRES(mp);
+
 }
 
+/*
+ * Growing the data section of the filesystem.
+ *	superblock
+ *	agi and agf
+ *	allocation btrees
+ */
 STATIC uint
-xfs_calc_growdata_reservation(xfs_mount_t *mp)
+xfs_calc_growdata_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_GROWDATA_LOG_RES(mp);
+	return mp->m_sb.sb_sectsize * 3 +
+		XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		128 * (3 + XFS_ALLOCFREE_LOG_COUNT(mp, 1));
 }
 
+/*
+ * Growing the rt section of the filesystem.
+ * In the first set of transactions (ALLOC) we allocate space to the
+ * bitmap or summary files.
+ *	superblock: sector size
+ *	agf of the ag from which the extent is allocated: sector size
+ *	bmap btree for bitmap/summary inode: max depth * blocksize
+ *	bitmap/summary inode: inode size
+ *	allocation btrees for 1 block alloc: 2 * (2 * maxdepth - 1) * blocksize
+ */
 STATIC uint
-xfs_calc_growrtalloc_reservation(xfs_mount_t *mp)
+xfs_calc_growrtalloc_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_GROWRTALLOC_LOG_RES(mp);
+	return 2 * mp->m_sb.sb_sectsize +
+		XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK)) +
+		mp->m_sb.sb_inodesize +
+		XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		(128 * (3 + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) +
+			XFS_ALLOCFREE_LOG_COUNT(mp, 1)));
 }
 
+/*
+ * Growing the rt section of the filesystem.
+ * In the second set of transactions (ZERO) we zero the new metadata blocks.
+ *	one bitmap/summary block: blocksize
+ */
 STATIC uint
-xfs_calc_growrtzero_reservation(xfs_mount_t *mp)
+xfs_calc_growrtzero_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_GROWRTZERO_LOG_RES(mp);
+	return mp->m_sb.sb_blocksize + 128;
 }
 
+/*
+ * Growing the rt section of the filesystem.
+ * In the third set of transactions (FREE) we update metadata without
+ * allocating any new blocks.
+ *	superblock: sector size
+ *	bitmap inode: inode size
+ *	summary inode: inode size
+ *	one bitmap block: blocksize
+ *	summary blocks: new summary size
+ */
 STATIC uint
-xfs_calc_growrtfree_reservation(xfs_mount_t *mp)
+xfs_calc_growrtfree_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_GROWRTFREE_LOG_RES(mp);
+	return mp->m_sb.sb_sectsize +
+		2 * mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_blocksize +
+		mp->m_rsumsize +
+		(128 * 5);
 }
 
+/*
+ * Logging the inode modification timestamp on a synchronous write.
+ *	inode
+ */
 STATIC uint
-xfs_calc_swrite_reservation(xfs_mount_t *mp)
+xfs_calc_swrite_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_SWRITE_LOG_RES(mp);
+	return mp->m_sb.sb_inodesize + 128;
 }
 
+/*
+ * Logging the inode mode bits when writing a setuid/setgid file
+ *	inode
+ */
 STATIC uint
 xfs_calc_writeid_reservation(xfs_mount_t *mp)
 {
-	return XFS_CALC_WRITEID_LOG_RES(mp);
+	return mp->m_sb.sb_inodesize + 128;
 }
 
+/*
+ * Converting the inode from non-attributed to attributed.
+ *	the inode being converted: inode size
+ *	agf block and superblock (for block allocation)
+ *	the new block (directory sized)
+ *	bmap blocks for the new directory block
+ *	allocation btrees
+ */
 STATIC uint
-xfs_calc_addafork_reservation(xfs_mount_t *mp)
+xfs_calc_addafork_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ADDAFORK_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_sectsize * 2 +
+		mp->m_dirblksize +
+		XFS_FSB_TO_B(mp, XFS_DAENTER_BMAP1B(mp, XFS_DATA_FORK) + 1) +
+		XFS_ALLOCFREE_LOG_RES(mp, 1) +
+		(128 * (4 + XFS_DAENTER_BMAP1B(mp, XFS_DATA_FORK) + 1 +
+			XFS_ALLOCFREE_LOG_COUNT(mp, 1))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * Removing the attribute fork of a file
+ *    the inode being truncated: inode size
+ *    the inode's bmap btree: max depth * block size
+ * And the bmap_finish transaction can free the blocks and bmap blocks:
+ *    the agf for each of the ags: 4 * sector size
+ *    the agfl for each of the ags: 4 * sector size
+ *    the super block to reflect the freed blocks: sector size
+ *    worst case split in allocation btrees per extent assuming 4 extents:
+ *		4 exts * 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_attrinval_reservation(xfs_mount_t *mp)
+xfs_calc_attrinval_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ATTRINVAL_LOG_RES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK)) +
+		 128 * (1 + XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK))),
+		(4 * mp->m_sb.sb_sectsize +
+		 4 * mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 4) +
+		 128 * (9 + XFS_ALLOCFREE_LOG_COUNT(mp, 4))));
 }
 
+/*
+ * Setting an attribute.
+ *	the inode getting the attribute
+ *	the superblock for allocations
+ *	the agfs extents are allocated from
+ *	the attribute btree * max depth
+ *	the inode allocation btree
+ * Since attribute transaction space is dependent on the size of the attribute,
+ * the calculation is done partially at mount time and partially at runtime.
+ */
 STATIC uint
-xfs_calc_attrset_reservation(xfs_mount_t *mp)
+xfs_calc_attrset_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ATTRSET_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_sectsize +
+		XFS_FSB_TO_B(mp, XFS_DA_NODE_MAXDEPTH) +
+		(128 * (2 + XFS_DA_NODE_MAXDEPTH)) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * Removing an attribute.
+ *    the inode: inode size
+ *    the attribute btree could join: max depth * block size
+ *    the inode bmap btree could join or split: max depth * block size
+ * And the bmap_finish transaction can free the attr blocks freed giving:
+ *    the agf for the ag in which the blocks live: 2 * sector size
+ *    the agfl for the ag in which the blocks live: 2 * sector size
+ *    the superblock for the free block count: sector size
+ *    the allocation btrees: 2 exts * 2 trees * (2 * max depth - 1) * block size
+ */
 STATIC uint
-xfs_calc_attrrm_reservation(xfs_mount_t *mp)
+xfs_calc_attrrm_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ATTRRM_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(
+		(mp->m_sb.sb_inodesize +
+		 XFS_FSB_TO_B(mp, XFS_DA_NODE_MAXDEPTH) +
+		 XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK)) +
+		 128 * (1 + XFS_DA_NODE_MAXDEPTH +
+		 	XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK))),
+		(2 * mp->m_sb.sb_sectsize +
+		 2 * mp->m_sb.sb_sectsize +
+		 mp->m_sb.sb_sectsize +
+		 XFS_ALLOCFREE_LOG_RES(mp, 2) +
+		 128 * (5 + XFS_ALLOCFREE_LOG_COUNT(mp, 2)))) +
+		XFS_DQUOT_LOGRES(mp);
 }
 
+/*
+ * Clearing a bad agino number in an agi hash bucket.
+ */
 STATIC uint
-xfs_calc_clear_agi_bucket_reservation(xfs_mount_t *mp)
+xfs_calc_clear_agi_bucket_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_CLEAR_AGI_BUCKET_LOG_RES(mp);
+	return mp->m_sb.sb_sectsize + 128;
 }
 
 /*
fs/xfs/xfs_trans.h +0 -411

453 lines not on this page · raw message

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 4 lines · Christoph Hellwig

Instead of having small helper functions calling big macros do the calculations for the log reservations directly in the functions. These are mostly 1:1 from the macros execept that the macros kept the quota calculations in their callers.

I like the idea, just a coupl eof small(ish) comments.

Firstly:

Review

Quoted 2 lines
(quoted diff) +1 -1
-	return XFS_CALC_WRITE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return MAX(

<some large expression>

fs/xfs/xfs_trans.c line 102

Quoted 1 line
fs/xfs/xfs_trans.c +1 -0
+		) + XFS_DQUOT_LOGRES(mp);

It's not very obvious that the dquot logres is separate to the large calculation. The single space indents as perenthesis are nested just doesn't make this stand out like I think it should. Can you put the XFS_DQUOT_LOGRES(mp) first in all these calculations so that it is more obvious that it is both present and separate to the main reservation?i

That would also address the second issue that the way you terminate the major expression and add the dquot logres is different in a couple of functions.

I'm also wondering if we should be converting some of the magic numbers like:

fs/xfs/xfs_trans.c line 343

Quoted 10 lines
fs/xfs/xfs_trans.c +7 -1
+xfs_calc_ichange_reservation(
+	struct xfs_mount	*mp)
 {
-	return XFS_CALC_ICHANGE_LOG_RES(mp) + XFS_DQUOT_LOGRES(mp);
+	return mp->m_sb.sb_inodesize +
+		mp->m_sb.sb_sectsize +
+		512 +
+		XFS_DQUOT_LOGRES(mp);
+
 }

to be correct. i.e.

	return mp->m_sb.sb_inodesize +
		mp->m_sb.sb_sectsize +
		sizeof(struct xfs_inode_log_format) +
		sizeof(struct xfs_buf_log_format) +
		XFS_DQUOT_LOGRES(mp);

instead of the magic 512 bytes that are added? I guess that's more of a followup correctness patch than a cleanup patch....

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs