Commit f2890730 authored by Jan Kara's avatar Jan Kara Committed by Theodore Ts'o
Browse files

ext4: Avoid unnecessary revokes in ext4_alloc_branch() 



Error cleanup path in ext4_alloc_branch() calls ext4_forget() on freshly
allocated indirect blocks with 'metadata' set to 1. This results in
generating revoke records for these blocks. However this is unnecessary
as the freed blocks are only allocated in the current transaction and
thus they will never be journalled. Make this cleanup path similar to
e.g. cleanup in ext4_splice_branch() and use ext4_free_blocks() to
handle block forgetting by passing EXT4_FREE_BLOCKS_FORGET and not
EXT4_FREE_BLOCKS_METADATA to ext4_free_blocks(). This also allows
allocating transaction not to reserve any credits for revoke records.

Reviewed-by: default avatarTheodore Ts'o <tytso@mit.edu>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20191105164437.32602-9-jack@suse.cz


Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 6cb367c2

fs/ext4/indirect.c

+19 −9
Original line number Diff line number Diff line
@@ -331,11 +331,14 @@ static int ext4_alloc_branch(handle_t *handle, 
	for (i = 0; i <= indirect_blks; i++) {

		if (i == indirect_blks) {

			new_blocks[i] = ext4_mb_new_blocks(handle, ar, &err);

		} else

		} else {

			ar->goal = new_blocks[i] = ext4_new_meta_blocks(handle,

					ar->inode, ar->goal,

					ar->flags & EXT4_MB_DELALLOC_RESERVED,

					NULL, &err);

			/* Simplify error cleanup... */

			branch[i+1].bh = NULL;

		}

		if (err) {

			i--;

			goto failed;
@@ -377,18 +380,25 @@ static int ext4_alloc_branch(handle_t *handle, 
	}

	return 0;

failed:

	if (i == indirect_blks) {

		/* Free data blocks */

		ext4_free_blocks(handle, ar->inode, NULL, new_blocks[i],

				 ar->len, 0);

		i--;

	}

	for (; i >= 0; i--) {

		/*

		 * We want to ext4_forget() only freshly allocated indirect

		 * blocks.  Buffer for new_blocks[i-1] is at branch[i].bh and

		 * buffer at branch[0].bh is indirect block / inode already

		 * existing before ext4_alloc_branch() was called.

		 * blocks. Buffer for new_blocks[i] is at branch[i+1].bh

		 * (buffer at branch[0].bh is indirect block / inode already

		 * existing before ext4_alloc_branch() was called). Also

		 * because blocks are freshly allocated, we don't need to

		 * revoke them which is why we don't set

		 * EXT4_FREE_BLOCKS_METADATA.

		 */

		if (i > 0 && i != indirect_blks && branch[i].bh)

			ext4_forget(handle, 1, ar->inode, branch[i].bh,

				    branch[i].bh->b_blocknr);

		ext4_free_blocks(handle, ar->inode, NULL, new_blocks[i],

				 (i == indirect_blks) ? ar->len : 1, 0);

		ext4_free_blocks(handle, ar->inode, branch[i+1].bh,

				 new_blocks[i], 1,

				 branch[i+1].bh ? EXT4_FREE_BLOCKS_FORGET : 0);

	}

	return err;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE