[PATCH 0/8] ext4: extents.c cleanups

[PATCH 0/8] ext4: extents.c cleanups

[Eric Biggers]

This series makes a few cleanups to things I noticed while reading some
of the code in extents.c.

No actual changes in behavior.

Eric Biggers (8):
  ext4: remove ext4_{ind,ext}_calc_metadata_amount()
  ext4: clean up len and offset checks in ext4_fallocate()
  ext4: remove redundant S_ISREG() checks from ext4_fallocate()
  ext4: make some functions static in extents.c
  ext4: fix documentation for ext4_ext_try_to_merge()
  ext4: remove obsolete comment from ext4_can_extents_be_merged()
  ext4: fix some nonstandard indentation in extents.c
  ext4: add missing braces in ext4_ext_drop_refs()

 fs/ext4/ext4.h         |  11 ----
 fs/ext4/ext4_extents.h |   5 --
 fs/ext4/extents.c      | 143 +++++++++++++----------------------------
 fs/ext4/indirect.c     |  26 --------
 fs/ext4/inode.c        |   3 -
 fs/ext4/super.c        |   2 -
 6 files changed, 45 insertions(+), 145 deletions(-)

-- 
2.24.1

[PATCH 1/8] ext4: remove ext4_{ind,ext}_calc_metadata_amount()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Remove the ext4_ind_calc_metadata_amount() and
ext4_ext_calc_metadata_amount() functions, which have been unused since
commit 71d4f7d03214 ("ext4: remove metadata reservation checks").

Also remove the i_da_metadata_calc_last_lblock and
i_da_metadata_calc_len fields from struct ext4_inode_info, as these were
only used by these removed functions.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/ext4.h     |  5 -----
 fs/ext4/extents.c  | 47 ----------------------------------------------
 fs/ext4/indirect.c | 26 -------------------------
 fs/ext4/super.c    |  2 --
 4 files changed, 80 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index f8578caba40d..0fdc913b0d6c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1052,8 +1052,6 @@ struct ext4_inode_info {
 	/* allocation reservation info for delalloc */
 	/* In case of bigalloc, this refer to clusters rather than blocks */
 	unsigned int i_reserved_data_blocks;
-	ext4_lblk_t i_da_metadata_calc_last_lblock;
-	int i_da_metadata_calc_len;
 
 	/* pending cluster reservations for bigalloc file systems */
 	struct ext4_pending_tree i_pending_tree;
@@ -2628,7 +2626,6 @@ extern int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk,
 /* indirect.c */
 extern int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 				struct ext4_map_blocks *map, int flags);
-extern int ext4_ind_calc_metadata_amount(struct inode *inode, sector_t lblock);
 extern int ext4_ind_trans_blocks(struct inode *inode, int nrblocks);
 extern void ext4_ind_truncate(handle_t *, struct inode *inode);
 extern int ext4_ind_remove_space(handle_t *handle, struct inode *inode,
@@ -3271,8 +3268,6 @@ extern int ext4_convert_unwritten_io_end_vec(handle_t *handle,
 					     ext4_io_end_t *io_end);
 extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
 			   struct ext4_map_blocks *map, int flags);
-extern int ext4_ext_calc_metadata_amount(struct inode *inode,
-					 ext4_lblk_t lblocks);
 extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
 						   int num,
 						   struct ext4_ext_path *path);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index fee19c9f5fe3..c6c89e38f43a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -309,53 +309,6 @@ ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
 			(nofail ? EXT4_GET_BLOCKS_METADATA_NOFAIL:0));
 }
 
-/*
- * Calculate the number of metadata blocks needed
- * to allocate @blocks
- * Worse case is one block per extent
- */
-int ext4_ext_calc_metadata_amount(struct inode *inode, ext4_lblk_t lblock)
-{
-	struct ext4_inode_info *ei = EXT4_I(inode);
-	int idxs;
-
-	idxs = ((inode->i_sb->s_blocksize - sizeof(struct ext4_extent_header))
-		/ sizeof(struct ext4_extent_idx));
-
-	/*
-	 * If the new delayed allocation block is contiguous with the
-	 * previous da block, it can share index blocks with the
-	 * previous block, so we only need to allocate a new index
-	 * block every idxs leaf blocks.  At ldxs**2 blocks, we need
-	 * an additional index block, and at ldxs**3 blocks, yet
-	 * another index blocks.
-	 */
-	if (ei->i_da_metadata_calc_len &&
-	    ei->i_da_metadata_calc_last_lblock+1 == lblock) {
-		int num = 0;
-
-		if ((ei->i_da_metadata_calc_len % idxs) == 0)
-			num++;
-		if ((ei->i_da_metadata_calc_len % (idxs*idxs)) == 0)
-			num++;
-		if ((ei->i_da_metadata_calc_len % (idxs*idxs*idxs)) == 0) {
-			num++;
-			ei->i_da_metadata_calc_len = 0;
-		} else
-			ei->i_da_metadata_calc_len++;
-		ei->i_da_metadata_calc_last_lblock++;
-		return num;
-	}
-
-	/*
-	 * In the worst case we need a new set of index blocks at
-	 * every level of the inode's extent tree.
-	 */
-	ei->i_da_metadata_calc_len = 1;
-	ei->i_da_metadata_calc_last_lblock = lblock;
-	return ext_depth(inode) + 1;
-}
-
 static int
 ext4_ext_max_entries(struct inode *inode, int depth)
 {
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 3a4ab70fe9e0..569fc68e8975 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -659,32 +659,6 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 	return err;
 }
 
-/*
- * Calculate the number of metadata blocks need to reserve
- * to allocate a new block at @lblocks for non extent file based file
- */
-int ext4_ind_calc_metadata_amount(struct inode *inode, sector_t lblock)
-{
-	struct ext4_inode_info *ei = EXT4_I(inode);
-	sector_t dind_mask = ~((sector_t)EXT4_ADDR_PER_BLOCK(inode->i_sb) - 1);
-	int blk_bits;
-
-	if (lblock < EXT4_NDIR_BLOCKS)
-		return 0;
-
-	lblock -= EXT4_NDIR_BLOCKS;
-
-	if (ei->i_da_metadata_calc_len &&
-	    (lblock & dind_mask) == ei->i_da_metadata_calc_last_lblock) {
-		ei->i_da_metadata_calc_len++;
-		return 0;
-	}
-	ei->i_da_metadata_calc_last_lblock = lblock & dind_mask;
-	ei->i_da_metadata_calc_len = 1;
-	blk_bits = order_base_2(lblock);
-	return (blk_bits / EXT4_ADDR_PER_BLOCK_BITS(inode->i_sb)) + 1;
-}
-
 /*
  * Calculate number of indirect blocks touched by mapping @nrblocks logically
  * contiguous blocks
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2937a8873fe1..518e74b93548 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1085,8 +1085,6 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
 	ei->i_es_shk_nr = 0;
 	ei->i_es_shrink_lblk = 0;
 	ei->i_reserved_data_blocks = 0;
-	ei->i_da_metadata_calc_len = 0;
-	ei->i_da_metadata_calc_last_lblock = 0;
 	spin_lock_init(&(ei->i_block_reservation_lock));
 	ext4_init_pending_tree(&ei->i_pending_tree);
 #ifdef CONFIG_QUOTA
-- 
2.24.1

[PATCH 2/8] ext4: clean up len and offset checks in ext4_fallocate()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

- Fix some comments.

- Consistently access i_size directly rather than using i_size_read(),
  since in all relevant cases we're under inode_lock().

- Simplify the alignment checks by using the IS_ALIGNED() macro.

- In ext4_insert_range(), do the check against s_maxbytes in a way
  that is safe against signed overflow.  (This doesn't currently matter
  for ext4 due to ext4's limited max file size, but this is something
  other filesystems have gotten wrong.  We might as well do it safely.)

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index c6c89e38f43a..21d39758d522 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4715,7 +4715,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
 	}
 
 	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
-	    (offset + len > i_size_read(inode) ||
+	    (offset + len > inode->i_size ||
 	     offset + len > EXT4_I(inode)->i_disksize)) {
 		new_size = offset + len;
 		ret = inode_newsize_ok(inode, new_size);
@@ -4799,7 +4799,7 @@ static long ext4_zero_range(struct file *file, loff_t offset,
 		* Mark that we allocate beyond EOF so the subsequent truncate
 		* can proceed even if the new size is the same as i_size.
 		*/
-		if ((offset + len) > i_size_read(inode))
+		if (offset + len > inode->i_size)
 			ext4_set_inode_flag(inode, EXT4_INODE_EOFBLOCKS);
 	}
 	ext4_mark_inode_dirty(handle, inode);
@@ -4886,7 +4886,7 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	}
 
 	if (!(mode & FALLOC_FL_KEEP_SIZE) &&
-	    (offset + len > i_size_read(inode) ||
+	    (offset + len > inode->i_size ||
 	     offset + len > EXT4_I(inode)->i_disksize)) {
 		new_size = offset + len;
 		ret = inode_newsize_ok(inode, new_size);
@@ -5434,9 +5434,8 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
 		return -EOPNOTSUPP;
 
-	/* Collapse range works only on fs block size aligned offsets. */
-	if (offset & (EXT4_CLUSTER_SIZE(sb) - 1) ||
-	    len & (EXT4_CLUSTER_SIZE(sb) - 1))
+	/* Collapse range works only on fs cluster size aligned regions. */
+	if (!IS_ALIGNED(offset | len, EXT4_CLUSTER_SIZE(sb)))
 		return -EINVAL;
 
 	if (!S_ISREG(inode->i_mode))
@@ -5459,7 +5458,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	 * There is no need to overlap collapse range with EOF, in which case
 	 * it is effectively a truncate operation
 	 */
-	if (offset + len >= i_size_read(inode)) {
+	if (offset + len >= inode->i_size) {
 		ret = -EINVAL;
 		goto out_mutex;
 	}
@@ -5537,7 +5536,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 		goto out_stop;
 	}
 
-	new_size = i_size_read(inode) - len;
+	new_size = inode->i_size - len;
 	i_size_write(inode, new_size);
 	EXT4_I(inode)->i_disksize = new_size;
 
@@ -5584,9 +5583,8 @@ int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
 		return -EOPNOTSUPP;
 
-	/* Insert range works only on fs block size aligned offsets. */
-	if (offset & (EXT4_CLUSTER_SIZE(sb) - 1) ||
-			len & (EXT4_CLUSTER_SIZE(sb) - 1))
+	/* Insert range works only on fs cluster size aligned regions. */
+	if (!IS_ALIGNED(offset | len, EXT4_CLUSTER_SIZE(sb)))
 		return -EINVAL;
 
 	if (!S_ISREG(inode->i_mode))
@@ -5611,14 +5609,14 @@ int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
 		goto out_mutex;
 	}
 
-	/* Check for wrap through zero */
-	if (inode->i_size + len > inode->i_sb->s_maxbytes) {
+	/* Check whether the maximum file size would be exceeded */
+	if (len > inode->i_sb->s_maxbytes - inode->i_size) {
 		ret = -EFBIG;
 		goto out_mutex;
 	}
 
-	/* Offset should be less than i_size */
-	if (offset >= i_size_read(inode)) {
+	/* Offset must be less than i_size */
+	if (offset >= inode->i_size) {
 		ret = -EINVAL;
 		goto out_mutex;
 	}
-- 
2.24.1

[PATCH 3/8] ext4: remove redundant S_ISREG() checks from ext4_fallocate()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

ext4_fallocate() is only used in the file_operations for regular files.
Also, the VFS only allows fallocate() on regular files and block
devices, but block devices always use blkdev_fallocate().  For both of
these reasons, S_ISREG() is always true in ext4_fallocate().

Therefore the S_ISREG() checks in ext4_zero_range(),
ext4_collapse_range(), ext4_insert_range(), and ext4_punch_hole() are
redundant.  Remove them.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 9 ---------
 fs/ext4/inode.c   | 3 ---
 2 files changed, 12 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 21d39758d522..296ad7fcc302 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4673,9 +4673,6 @@ static long ext4_zero_range(struct file *file, loff_t offset,
 
 	trace_ext4_zero_range(inode, offset, len, mode);
 
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
-
 	/* Call ext4_force_commit to flush all data in case of data=journal. */
 	if (ext4_should_journal_data(inode)) {
 		ret = ext4_force_commit(inode->i_sb);
@@ -5438,9 +5435,6 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	if (!IS_ALIGNED(offset | len, EXT4_CLUSTER_SIZE(sb)))
 		return -EINVAL;
 
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
-
 	trace_ext4_collapse_range(inode, offset, len);
 
 	punch_start = offset >> EXT4_BLOCK_SIZE_BITS(sb);
@@ -5587,9 +5581,6 @@ int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
 	if (!IS_ALIGNED(offset | len, EXT4_CLUSTER_SIZE(sb)))
 		return -EINVAL;
 
-	if (!S_ISREG(inode->i_mode))
-		return -EOPNOTSUPP;
-
 	trace_ext4_insert_range(inode, offset, len);
 
 	offset_lblk = offset >> EXT4_BLOCK_SIZE_BITS(sb);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 6586b29e9f2f..32ab4d4074be 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3916,9 +3916,6 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
 	unsigned int credits;
 	int ret = 0;
 
-	if (!S_ISREG(inode->i_mode))
-		return -EOPNOTSUPP;
-
 	trace_ext4_punch_hole(inode, offset, length, 0);
 
 	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
-- 
2.24.1

[PATCH 4/8] ext4: make some functions static in extents.c

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Make the following functions static since they're only used in
extents.c:

	__ext4_ext_dirty()
	ext4_can_extents_be_merged()
	ext4_collapse_range()
	ext4_insert_range()

Also remove the prototype for ext4_ext_writepage_trans_blocks(), as this
function is not defined anywhere.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/ext4.h         |  6 ------
 fs/ext4/ext4_extents.h |  5 -----
 fs/ext4/extents.c      | 22 +++++++++++++++-------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 0fdc913b0d6c..9b395a2e1cd9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3251,7 +3251,6 @@ struct ext4_extent;
 #define EXT_MAX_BLOCKS	0xffffffff
 
 extern int ext4_ext_tree_init(handle_t *handle, struct inode *);
-extern int ext4_ext_writepage_trans_blocks(struct inode *, int);
 extern int ext4_ext_index_trans_blocks(struct inode *inode, int extents);
 extern int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 			       struct ext4_map_blocks *map, int flags);
@@ -3271,9 +3270,6 @@ extern int ext4_map_blocks(handle_t *handle, struct inode *inode,
 extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
 						   int num,
 						   struct ext4_ext_path *path);
-extern int ext4_can_extents_be_merged(struct inode *inode,
-				      struct ext4_extent *ex1,
-				      struct ext4_extent *ex2);
 extern int ext4_ext_insert_extent(handle_t *, struct inode *,
 				  struct ext4_ext_path **,
 				  struct ext4_extent *, int);
@@ -3289,8 +3285,6 @@ extern int ext4_get_es_cache(struct inode *inode,
 			     struct fiemap_extent_info *fieinfo,
 			     __u64 start, __u64 len);
 extern int ext4_ext_precache(struct inode *inode);
-extern int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
-extern int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
 extern int ext4_swap_extents(handle_t *handle, struct inode *inode1,
 				struct inode *inode2, ext4_lblk_t lblk1,
 			     ext4_lblk_t lblk2,  ext4_lblk_t count,
diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index 98bd0e9ee7df..1c216fcc202a 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -267,10 +267,5 @@ static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix,
 				     0xffff);
 }
 
-#define ext4_ext_dirty(handle, inode, path) \
-		__ext4_ext_dirty(__func__, __LINE__, (handle), (inode), (path))
-int __ext4_ext_dirty(const char *where, unsigned int line, handle_t *handle,
-		     struct inode *inode, struct ext4_ext_path *path);
-
 #endif /* _EXT4_EXTENTS */
 
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 296ad7fcc302..6c3f8c0ca83b 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -161,8 +161,9 @@ static int ext4_ext_get_access(handle_t *handle, struct inode *inode,
  *  - ENOMEM
  *  - EIO
  */
-int __ext4_ext_dirty(const char *where, unsigned int line, handle_t *handle,
-		     struct inode *inode, struct ext4_ext_path *path)
+static int __ext4_ext_dirty(const char *where, unsigned int line,
+			    handle_t *handle, struct inode *inode,
+			    struct ext4_ext_path *path)
 {
 	int err;
 
@@ -179,6 +180,9 @@ int __ext4_ext_dirty(const char *where, unsigned int line, handle_t *handle,
 	return err;
 }
 
+#define ext4_ext_dirty(handle, inode, path) \
+		__ext4_ext_dirty(__func__, __LINE__, (handle), (inode), (path))
+
 static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode,
 			      struct ext4_ext_path *path,
 			      ext4_lblk_t block)
@@ -1695,9 +1699,9 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
 	return err;
 }
 
-int
-ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1,
-				struct ext4_extent *ex2)
+static int ext4_can_extents_be_merged(struct inode *inode,
+				      struct ext4_extent *ex1,
+				      struct ext4_extent *ex2)
 {
 	unsigned short ext1_ee_len, ext2_ee_len;
 
@@ -4656,6 +4660,10 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
 	return ret > 0 ? ret2 : ret;
 }
 
+static int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len);
+
+static int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len);
+
 static long ext4_zero_range(struct file *file, loff_t offset,
 			    loff_t len, int mode)
 {
@@ -5414,7 +5422,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
  * This implements the fallocate's collapse range functionality for ext4
  * Returns: 0 and non-zero on error.
  */
-int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
+static int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 {
 	struct super_block *sb = inode->i_sb;
 	ext4_lblk_t punch_start, punch_stop;
@@ -5558,7 +5566,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
  * by len bytes.
  * Returns 0 on success, error otherwise.
  */
-int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
+static int ext4_insert_range(struct inode *inode, loff_t offset, loff_t len)
 {
 	struct super_block *sb = inode->i_sb;
 	handle_t *handle;
-- 
2.24.1

[PATCH 5/8] ext4: fix documentation for ext4_ext_try_to_merge()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Don't mention the nonexistent return value, and mention both types of
merges that are attempted.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 6c3f8c0ca83b..b2dee02ed238 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1827,13 +1827,14 @@ static void ext4_ext_try_to_merge_up(handle_t *handle,
 }
 
 /*
- * This function tries to merge the @ex extent to neighbours in the tree.
- * return 1 if merge left else 0.
+ * This function tries to merge the @ex extent to neighbours in the tree, then
+ * tries to collapse the extent tree into the inode.
  */
 static void ext4_ext_try_to_merge(handle_t *handle,
 				  struct inode *inode,
 				  struct ext4_ext_path *path,
-				  struct ext4_extent *ex) {
+				  struct ext4_extent *ex)
+{
 	struct ext4_extent_header *eh;
 	unsigned int depth;
 	int merge_done = 0;
-- 
2.24.1

[PATCH 6/8] ext4: remove obsolete comment from ext4_can_extents_be_merged()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Support for unwritten extents was added to ext4 a long time ago, so
remove a misleading comment that says they're a future feature.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index b2dee02ed238..25e6e83cdeca 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1715,11 +1715,6 @@ static int ext4_can_extents_be_merged(struct inode *inode,
 			le32_to_cpu(ex2->ee_block))
 		return 0;
 
-	/*
-	 * To allow future support for preallocated extents to be added
-	 * as an RO_COMPAT feature, refuse to merge to extents if
-	 * this can result in the top bit of ee_len being set.
-	 */
 	if (ext1_ee_len + ext2_ee_len > EXT_INIT_MAX_LEN)
 		return 0;
 
-- 
2.24.1

[PATCH 7/8] ext4: fix some nonstandard indentation in extents.c

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Clean up some code that was using 2-character indents.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 25e6e83cdeca..1cdcd9b0934d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -606,8 +606,9 @@ static void ext4_ext_show_path(struct inode *inode, struct ext4_ext_path *path)
 	ext_debug("path:");
 	for (k = 0; k <= l; k++, path++) {
 		if (path->p_idx) {
-		  ext_debug("  %d->%llu", le32_to_cpu(path->p_idx->ei_block),
-			    ext4_idx_pblock(path->p_idx));
+			ext_debug("  %d->%llu",
+				  le32_to_cpu(path->p_idx->ei_block),
+				  ext4_idx_pblock(path->p_idx));
 		} else if (path->p_ext) {
 			ext_debug("  %d:[%d]%d:%llu ",
 				  le32_to_cpu(path->p_ext->ee_block),
@@ -734,8 +735,8 @@ ext4_ext_binsearch_idx(struct inode *inode,
 
 		chix = ix = EXT_FIRST_INDEX(eh);
 		for (k = 0; k < le16_to_cpu(eh->eh_entries); k++, ix++) {
-		  if (k != 0 &&
-		      le32_to_cpu(ix->ei_block) <= le32_to_cpu(ix[-1].ei_block)) {
+			if (k != 0 && le32_to_cpu(ix->ei_block) <=
+			    le32_to_cpu(ix[-1].ei_block)) {
 				printk(KERN_DEBUG "k=%d, ix=0x%p, "
 				       "first=0x%p\n", k,
 				       ix, EXT_FIRST_INDEX(eh));
@@ -1589,17 +1590,16 @@ ext4_ext_next_allocated_block(struct ext4_ext_path *path)
 		return EXT_MAX_BLOCKS;
 
 	while (depth >= 0) {
+		struct ext4_ext_path *p = &path[depth];
+
 		if (depth == path->p_depth) {
 			/* leaf */
-			if (path[depth].p_ext &&
-				path[depth].p_ext !=
-					EXT_LAST_EXTENT(path[depth].p_hdr))
-			  return le32_to_cpu(path[depth].p_ext[1].ee_block);
+			if (p->p_ext && p->p_ext != EXT_LAST_EXTENT(p->p_hdr))
+				return le32_to_cpu(p->p_ext[1].ee_block);
 		} else {
 			/* index */
-			if (path[depth].p_idx !=
-					EXT_LAST_INDEX(path[depth].p_hdr))
-			  return le32_to_cpu(path[depth].p_idx[1].ei_block);
+			if (p->p_idx != EXT_LAST_INDEX(p->p_hdr))
+				return le32_to_cpu(p->p_idx[1].ei_block);
 		}
 		depth--;
 	}
-- 
2.24.1

[PATCH 8/8] ext4: add missing braces in ext4_ext_drop_refs()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

For clarity, add braces to the loop in ext4_ext_drop_refs().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/extents.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 1cdcd9b0934d..640c3681dcc6 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -689,11 +689,12 @@ void ext4_ext_drop_refs(struct ext4_ext_path *path)
 	if (!path)
 		return;
 	depth = path->p_depth;
-	for (i = 0; i <= depth; i++, path++)
+	for (i = 0; i <= depth; i++, path++) {
 		if (path->p_bh) {
 			brelse(path->p_bh);
 			path->p_bh = NULL;
 		}
+	}
 }
 
 /*
-- 
2.24.1

[PATCH 1/1] ext4: remove unsed macro MPAGE_DA_EXTENT_TAIL

[Ritesh Harjani]

Remove unsed macro MPAGE_DA_EXTENT_TAIL which
is no more used after below commit
4e7ea81d ("ext4: restructure writeback path")

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 fs/ext4/inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 748a9e6baab1..b1249e82e57c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -48,8 +48,6 @@
 
 #include <trace/events/ext4.h>
 
-#define MPAGE_DA_EXTENT_TAIL 0x01
-
 static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
 			      struct ext4_inode_info *ei)
 {
-- 
2.21.0

Re: [PATCH 1/1] ext4: remove unsed macro MPAGE_DA_EXTENT_TAIL

[Ritesh Harjani]

NACK. Ignore this.
Will correct the spelling mistake in commit subject (unused)
and will resend it.

-riteshh

On 1/1/20 3:06 PM, Ritesh Harjani wrote:
> Remove unsed macro MPAGE_DA_EXTENT_TAIL which
> is no more used after below commit
> 4e7ea81d ("ext4: restructure writeback path")
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>   fs/ext4/inode.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 748a9e6baab1..b1249e82e57c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -48,8 +48,6 @@
>   
>   #include <trace/events/ext4.h>
>   
> -#define MPAGE_DA_EXTENT_TAIL 0x01
> -
>   static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
>   			      struct ext4_inode_info *ei)
>   {
> 

[PATCH 1/1] ext4: remove unused macro MPAGE_DA_EXTENT_TAIL

[Ritesh Harjani]

Remove unused macro MPAGE_DA_EXTENT_TAIL which
is no more used after below commit
4e7ea81d ("ext4: restructure writeback path")

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 fs/ext4/inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 748a9e6baab1..b1249e82e57c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -48,8 +48,6 @@
 
 #include <trace/events/ext4.h>
 
-#define MPAGE_DA_EXTENT_TAIL 0x01
-
 static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
 			      struct ext4_inode_info *ei)
 {
-- 
2.21.0

Re: [PATCH 0/8] ext4: extents.c cleanups

[Ritesh Harjani]

On 12/31/19 11:34 PM, Eric Biggers wrote:
> This series makes a few cleanups to things I noticed while reading some
> of the code in extents.c.
> 
> No actual changes in behavior.
> 
> Eric Biggers (8):
>    ext4: remove ext4_{ind,ext}_calc_metadata_amount()
>    ext4: clean up len and offset checks in ext4_fallocate()
>    ext4: remove redundant S_ISREG() checks from ext4_fallocate()
>    ext4: make some functions static in extents.c
>    ext4: fix documentation for ext4_ext_try_to_merge()
>    ext4: remove obsolete comment from ext4_can_extents_be_merged()
>    ext4: fix some nonstandard indentation in extents.c
>    ext4: add missing braces in ext4_ext_drop_refs()
>  >   fs/ext4/ext4.h         |  11 ----
>   fs/ext4/ext4_extents.h |   5 --
>   fs/ext4/extents.c      | 143 +++++++++++++----------------------------
>   fs/ext4/indirect.c     |  26 --------
>   fs/ext4/inode.c        |   3 -
>   fs/ext4/super.c        |   2 -
>   6 files changed, 45 insertions(+), 145 deletions(-)


Nice cleanup.

While reviewing your patch series also found an unused macro
"MPAGE_DA_EXTENT_TAIL" in inode.c file. Submitted a patch on top of
this series itself. Maybe it can be included as part of your extent
cleanup series itself.

Patch:- "ext4: remove unused macro MPAGE_DA_EXTENT_TAIL"

In your series I think that last 4 commits could be squashed into 1
commit itself, since those 4 patches are related to comments or
minor indentations in extents.c file. But no major objection on that.

Either ways, you may add.

Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>


-ritesh

Re: [PATCH 1/1] ext4: remove unused macro MPAGE_DA_EXTENT_TAIL

[Jan Kara]

On Wed 01-01-20 15:21:37, Ritesh Harjani wrote:
> Remove unused macro MPAGE_DA_EXTENT_TAIL which
> is no more used after below commit
> 4e7ea81d ("ext4: restructure writeback path")
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza


> ---
>  fs/ext4/inode.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 748a9e6baab1..b1249e82e57c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -48,8 +48,6 @@
>  
>  #include <trace/events/ext4.h>
>  
> -#define MPAGE_DA_EXTENT_TAIL 0x01
> -
>  static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
>  			      struct ext4_inode_info *ei)
>  {
> -- 
> 2.21.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/8] ext4: extents.c cleanups

[Jan Kara]

On Tue 31-12-19 12:04:36, Eric Biggers wrote:
> This series makes a few cleanups to things I noticed while reading some
> of the code in extents.c.
> 
> No actual changes in behavior.

All the cleanups look good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Eric Biggers (8):
>   ext4: remove ext4_{ind,ext}_calc_metadata_amount()
>   ext4: clean up len and offset checks in ext4_fallocate()
>   ext4: remove redundant S_ISREG() checks from ext4_fallocate()
>   ext4: make some functions static in extents.c
>   ext4: fix documentation for ext4_ext_try_to_merge()
>   ext4: remove obsolete comment from ext4_can_extents_be_merged()
>   ext4: fix some nonstandard indentation in extents.c
>   ext4: add missing braces in ext4_ext_drop_refs()
> 
>  fs/ext4/ext4.h         |  11 ----
>  fs/ext4/ext4_extents.h |   5 --
>  fs/ext4/extents.c      | 143 +++++++++++++----------------------------
>  fs/ext4/indirect.c     |  26 --------
>  fs/ext4/inode.c        |   3 -
>  fs/ext4/super.c        |   2 -
>  6 files changed, 45 insertions(+), 145 deletions(-)
> 
> -- 
> 2.24.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/8] ext4: extents.c cleanups

[Theodore Y. Ts'o]

On Tue, Dec 31, 2019 at 12:04:36PM -0600, Eric Biggers wrote:
> This series makes a few cleanups to things I noticed while reading some
> of the code in extents.c.
> 
> No actual changes in behavior.
> 
> Eric Biggers (8):
>   ext4: remove ext4_{ind,ext}_calc_metadata_amount()
>   ext4: clean up len and offset checks in ext4_fallocate()
>   ext4: remove redundant S_ISREG() checks from ext4_fallocate()
>   ext4: make some functions static in extents.c
>   ext4: fix documentation for ext4_ext_try_to_merge()
>   ext4: remove obsolete comment from ext4_can_extents_be_merged()
>   ext4: fix some nonstandard indentation in extents.c
>   ext4: add missing braces in ext4_ext_drop_refs()
> 
>  fs/ext4/ext4.h         |  11 ----
>  fs/ext4/ext4_extents.h |   5 --
>  fs/ext4/extents.c      | 143 +++++++++++++----------------------------
>  fs/ext4/indirect.c     |  26 --------
>  fs/ext4/inode.c        |   3 -
>  fs/ext4/super.c        |   2 -
>  6 files changed, 45 insertions(+), 145 deletions(-)

Thanks for the cleanup patches, and thanks to Ritesh and Jan for
reviewing them!  I've applied them to the ext4.git tree.

	  	      	      	      - Ted

Re: [PATCH 1/1] ext4: remove unused macro MPAGE_DA_EXTENT_TAIL

[Theodore Y. Ts'o]

On Tue, Jan 07, 2020 at 10:55:51AM +0100, Jan Kara wrote:
> On Wed 01-01-20 15:21:37, Ritesh Harjani wrote:
> > Remove unused macro MPAGE_DA_EXTENT_TAIL which
> > is no more used after below commit
> > 4e7ea81d ("ext4: restructure writeback path")
> > 
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> 
> Looks good to me. You can add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted