Commit fe4c224a authored Mar 07, 2014 by Dave Chinner Committed by Dave Chinner Mar 07, 2014

xfs: inode log reservations are still too small



Back in commit 23956703 ("xfs: inode log reservations are too
small"), the reservation size was increased to take into account the
difference in size between the in-memory BMBT block headers and the
on-disk BMDR headers. This solved a transaction overrun when logging
the inode size.

Recently, however, we've seen a number of these same overruns on
kernels with the above fix in it. All of them have been by 4 bytes,
so we must still not be accounting for something correctly.

Through inspection it turns out the above commit didn't take into
account everything it should have. That is, it only accounts for a
single log op_hdr structure, when it can actually require up to four
op_hdrs - one for each region (log iovec) that is formatted. These
regions are the inode log format header, the inode core, and the two
forks that can be held in the literal area of the inode.

This means we are not accounting for 36 bytes of log space that the
transaction can use, and hence when we get inodes in certain formats
with particular fragmentation patterns we can overrun the
transaction. Fix this by adding the correct accounting for log
op_headers in the transaction.

Tested-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>

parent a49935f2

fs/xfs/xfs_trans_resv.c

+15 −7

Original line number	Diff line number	Diff line
		@@ -81,17 +81,25 @@ xfs_calc_buf_res(
		* on disk. Hence we need an inode reservation function that calculates all this
		* correctly. So, we log:
		*
		* - log op headers for object
		* - 4 log op headers for object
		* - for the ilf, the inode core and 2 forks
		* - inode log format object
		* - the entire inode contents (core + 2 forks)
		* - two bmap btree block headers
		* - the inode core
		* - two inode forks containing bmap btree root blocks.
		* - the btree data contained by both forks will fit into the inode size,
		* hence when combined with the inode core above, we have a total of the
		* actual inode size.
		* - the BMBT headers need to be accounted separately, as they are
		* additional to the records and pointers that fit inside the inode
		* forks.
		*/
		STATIC uint
		xfs_calc_inode_res(
		struct xfs_mount *mp,
		uint ninodes)
		{
		return ninodes * (sizeof(struct xlog_op_header) +
		return ninodes *
		(4 * sizeof(struct xlog_op_header) +
		sizeof(struct xfs_inode_log_format) +
		mp->m_sb.sb_inodesize +
		2 * XFS_BMBT_BLOCK_LEN(mp));

Admin message