This series contains some random cleanups to mballoc. No function change
is intended except patch 8 may fix a potential memleak if non-used
preallocation spaces of inode could be greater than UNIT_MAX.
More details can be found in respective patches. Thanks!
v1->v2:
-Collect RVB from Jan.
-Remove 'needed' in trace_ext4_discard_preallocations.
Kemeng Shi (9):
ext4: remove unused return value of __mb_check_buddy
ext4: remove unused parameter group in ext4_mb_choose_next_group_*()
ext4: remove unneeded return value of ext4_mb_release_context
ext4: remove unused ext4_allocation_context::ac_groups_considered
ext4: remove unused return value of ext4_mb_release
ext4: remove unused return value of ext4_mb_release_inode_pa
ext4: remove unused return value of ext4_mb_release_group_pa
ext4: remove unnecessary parameter "needed" in
ext4_discard_preallocations
ext4: remove 'needed' in trace_ext4_discard_preallocations
fs/ext4/ext4.h | 4 +--
fs/ext4/extents.c | 10 ++++----
fs/ext4/file.c | 2 +-
fs/ext4/indirect.c | 2 +-
fs/ext4/inode.c | 6 ++---
fs/ext4/ioctl.c | 2 +-
fs/ext4/mballoc.c | 49 ++++++++++++++-----------------------
fs/ext4/mballoc.h | 1 -
fs/ext4/move_extent.c | 4 +--
fs/ext4/super.c | 2 +-
include/trace/events/ext4.h | 11 +++------
11 files changed, 38 insertions(+), 55 deletions(-)
--
2.30.0
Remove unused ext4_allocation_context::ac_groups_considered
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
index d7aeb5da7..56938532b 100644
--- a/fs/ext4/mballoc.h
+++ b/fs/ext4/mballoc.h
@@ -192,7 +192,6 @@ struct ext4_allocation_context {
*/
ext4_grpblk_t ac_orig_goal_len;
- __u32 ac_groups_considered;
__u32 ac_flags; /* allocation hints */
__u16 ac_groups_scanned;
__u16 ac_groups_linear_remaining;
--
2.30.0
Remove unused return value of ext4_mb_release_group_pa.
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 1ed6d1d5a..e6561a09d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5330,7 +5330,7 @@ ext4_mb_release_inode_pa(struct ext4_buddy *e4b, struct buffer_head *bitmap_bh,
atomic_add(free, &sbi->s_mb_discarded);
}
-static noinline_for_stack int
+static noinline_for_stack void
ext4_mb_release_group_pa(struct ext4_buddy *e4b,
struct ext4_prealloc_space *pa)
{
@@ -5344,13 +5344,11 @@ ext4_mb_release_group_pa(struct ext4_buddy *e4b,
if (unlikely(group != e4b->bd_group && pa->pa_len != 0)) {
ext4_warning(sb, "bad group: expected %u, group %u, pa_start %llu",
e4b->bd_group, group, pa->pa_pstart);
- return 0;
+ return;
}
mb_free_blocks(pa->pa_inode, e4b, bit, pa->pa_len);
atomic_add(pa->pa_len, &EXT4_SB(sb)->s_mb_discarded);
trace_ext4_mballoc_discard(sb, NULL, group, bit, pa->pa_len);
-
- return 0;
}
/*
--
2.30.0
Remove unused return value of ext4_mb_release.
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/ext4.h | 2 +-
fs/ext4/mballoc.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8ccebe0d4..fb35cae16 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2902,7 +2902,7 @@ extern const struct seq_operations ext4_mb_seq_groups_ops;
extern const struct seq_operations ext4_mb_seq_structs_summary_ops;
extern int ext4_seq_mb_stats_show(struct seq_file *seq, void *offset);
extern int ext4_mb_init(struct super_block *);
-extern int ext4_mb_release(struct super_block *);
+extern void ext4_mb_release(struct super_block *);
extern ext4_fsblk_t ext4_mb_new_blocks(handle_t *,
struct ext4_allocation_request *, int *);
extern void ext4_discard_preallocations(struct inode *, unsigned int);
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index f79e87ad3..8d6246db3 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3725,7 +3725,7 @@ static int ext4_mb_cleanup_pa(struct ext4_group_info *grp)
return count;
}
-int ext4_mb_release(struct super_block *sb)
+void ext4_mb_release(struct super_block *sb)
{
ext4_group_t ngroups = ext4_get_groups_count(sb);
ext4_group_t i;
@@ -3801,8 +3801,6 @@ int ext4_mb_release(struct super_block *sb)
}
free_percpu(sbi->s_locality_groups);
-
- return 0;
}
static inline int ext4_issue_discard(struct super_block *sb,
--
2.30.0
Function ext4_mb_release_context always return 0 and the return value is
never used. Just remove unneeded return value of ext4_mb_release_context.
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 765b62729..f79e87ad3 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5939,7 +5939,7 @@ static void ext4_mb_add_n_trim(struct ext4_allocation_context *ac)
/*
* release all resource we used in allocation
*/
-static int ext4_mb_release_context(struct ext4_allocation_context *ac)
+static void ext4_mb_release_context(struct ext4_allocation_context *ac)
{
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_prealloc_space *pa = ac->ac_pa;
@@ -5976,7 +5976,6 @@ static int ext4_mb_release_context(struct ext4_allocation_context *ac)
if (ac->ac_flags & EXT4_MB_HINT_GROUP_ALLOC)
mutex_unlock(&ac->ac_lg->lg_mutex);
ext4_mb_collect_stats(ac);
- return 0;
}
static int ext4_mb_discard_preallocations(struct super_block *sb, int needed)
--
2.30.0
Remove unused return value of __mb_check_buddy.
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 454d56126..9f9b8dd06 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -677,7 +677,7 @@ do { \
} \
} while (0)
-static int __mb_check_buddy(struct ext4_buddy *e4b, char *file,
+static void __mb_check_buddy(struct ext4_buddy *e4b, char *file,
const char *function, int line)
{
struct super_block *sb = e4b->bd_sb;
@@ -696,7 +696,7 @@ static int __mb_check_buddy(struct ext4_buddy *e4b, char *file,
void *buddy2;
if (e4b->bd_info->bb_check_counter++ % 10)
- return 0;
+ return;
while (order > 1) {
buddy = mb_find_buddy(e4b, order, &max);
@@ -758,7 +758,7 @@ static int __mb_check_buddy(struct ext4_buddy *e4b, char *file,
grp = ext4_get_group_info(sb, e4b->bd_group);
if (!grp)
- return NULL;
+ return;
list_for_each(cur, &grp->bb_prealloc_list) {
ext4_group_t groupnr;
struct ext4_prealloc_space *pa;
@@ -768,7 +768,6 @@ static int __mb_check_buddy(struct ext4_buddy *e4b, char *file,
for (i = 0; i < pa->pa_len; i++)
MB_CHECK_ASSERT(mb_test_bit(k + i, buddy));
}
- return 0;
}
#undef MB_CHECK_ASSERT
#define mb_check_buddy(e4b) __mb_check_buddy(e4b, \
--
2.30.0
Remove unused parameter group in ext4_mb_choose_next_group_*().
Signed-off-by: Kemeng Shi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 9f9b8dd06..765b62729 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -870,7 +870,7 @@ mb_update_avg_fragment_size(struct super_block *sb, struct ext4_group_info *grp)
* cr level needs an update.
*/
static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context *ac,
- enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
+ enum criteria *new_cr, ext4_group_t *group)
{
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_group_info *iter;
@@ -944,7 +944,7 @@ ext4_mb_find_good_group_avg_frag_lists(struct ext4_allocation_context *ac, int o
* order. Updates *new_cr if cr level needs an update.
*/
static void ext4_mb_choose_next_group_goal_fast(struct ext4_allocation_context *ac,
- enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
+ enum criteria *new_cr, ext4_group_t *group)
{
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_group_info *grp = NULL;
@@ -989,7 +989,7 @@ static void ext4_mb_choose_next_group_goal_fast(struct ext4_allocation_context *
* much and fall to CR_GOAL_LEN_SLOW in that case.
*/
static void ext4_mb_choose_next_group_best_avail(struct ext4_allocation_context *ac,
- enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
+ enum criteria *new_cr, ext4_group_t *group)
{
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_group_info *grp = NULL;
@@ -1124,11 +1124,11 @@ static void ext4_mb_choose_next_group(struct ext4_allocation_context *ac,
}
if (*new_cr == CR_POWER2_ALIGNED) {
- ext4_mb_choose_next_group_p2_aligned(ac, new_cr, group, ngroups);
+ ext4_mb_choose_next_group_p2_aligned(ac, new_cr, group);
} else if (*new_cr == CR_GOAL_LEN_FAST) {
- ext4_mb_choose_next_group_goal_fast(ac, new_cr, group, ngroups);
+ ext4_mb_choose_next_group_goal_fast(ac, new_cr, group);
} else if (*new_cr == CR_BEST_AVAIL_LEN) {
- ext4_mb_choose_next_group_best_avail(ac, new_cr, group, ngroups);
+ ext4_mb_choose_next_group_best_avail(ac, new_cr, group);
} else {
/*
* TODO: For CR=2, we can arrange groups in an rb tree sorted by
--
2.30.0
As 'needed' to trace_ext4_discard_preallocations is always 0 which
is meaningless. Just remove it.
Signed-off-by: Kemeng Shi <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/ext4/mballoc.c | 5 ++---
include/trace/events/ext4.h | 11 ++++-------
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 0e6beb3b4..091a832a8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5481,9 +5481,8 @@ void ext4_discard_preallocations(struct inode *inode)
struct rb_node *iter;
int err;
- if (!S_ISREG(inode->i_mode)) {
+ if (!S_ISREG(inode->i_mode))
return;
- }
if (EXT4_SB(sb)->s_mount_state & EXT4_FC_REPLAY)
return;
@@ -5491,7 +5490,7 @@ void ext4_discard_preallocations(struct inode *inode)
mb_debug(sb, "discard preallocation for inode %lu\n",
inode->i_ino);
trace_ext4_discard_preallocations(inode,
- atomic_read(&ei->i_prealloc_active), 0);
+ atomic_read(&ei->i_prealloc_active));
repeat:
/* first, collect all pa's in the inode */
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index 65029dfb9..a697f4b77 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -772,15 +772,14 @@ TRACE_EVENT(ext4_mb_release_group_pa,
);
TRACE_EVENT(ext4_discard_preallocations,
- TP_PROTO(struct inode *inode, unsigned int len, unsigned int needed),
+ TP_PROTO(struct inode *inode, unsigned int len),
- TP_ARGS(inode, len, needed),
+ TP_ARGS(inode, len),
TP_STRUCT__entry(
__field( dev_t, dev )
__field( ino_t, ino )
__field( unsigned int, len )
- __field( unsigned int, needed )
),
@@ -788,13 +787,11 @@ TRACE_EVENT(ext4_discard_preallocations,
__entry->dev = inode->i_sb->s_dev;
__entry->ino = inode->i_ino;
__entry->len = len;
- __entry->needed = needed;
),
- TP_printk("dev %d,%d ino %lu len: %u needed %u",
+ TP_printk("dev %d,%d ino %lu len: %u",
MAJOR(__entry->dev), MINOR(__entry->dev),
- (unsigned long) __entry->ino, __entry->len,
- __entry->needed)
+ (unsigned long) __entry->ino, __entry->len)
);
TRACE_EVENT(ext4_mb_discard_preallocations,
--
2.30.0
The "needed" controls the number of ext4_prealloc_space to discard in
ext4_discard_preallocations. Function ext4_discard_preallocations is
supposed to discard all non-used preallocated blocks when "needed"
is 0 and now ext4_discard_preallocations is always called with "needed"
= 0. Remove unnecessary parameter "needed" and remove all non-used
preallocated spaces in ext4_discard_preallocations to simplify the
code.
Note: If count of non-used preallocated spaces could be more than
UINT_MAX, there was a memory leak as some non-used preallocated
spaces are left ununsed and this commit will fix it. Otherwise,
there is no behavior change.
Signed-off-by: Kemeng Shi <[email protected]>
---
fs/ext4/ext4.h | 2 +-
fs/ext4/extents.c | 10 +++++-----
fs/ext4/file.c | 2 +-
fs/ext4/indirect.c | 2 +-
fs/ext4/inode.c | 6 +++---
fs/ext4/ioctl.c | 2 +-
fs/ext4/mballoc.c | 10 +++-------
fs/ext4/move_extent.c | 4 ++--
fs/ext4/super.c | 2 +-
9 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index fb35cae16..2fd444034 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2905,7 +2905,7 @@ extern int ext4_mb_init(struct super_block *);
extern void ext4_mb_release(struct super_block *);
extern ext4_fsblk_t ext4_mb_new_blocks(handle_t *,
struct ext4_allocation_request *, int *);
-extern void ext4_discard_preallocations(struct inode *, unsigned int);
+extern void ext4_discard_preallocations(struct inode *);
extern int __init ext4_init_mballoc(void);
extern void ext4_exit_mballoc(void);
extern ext4_group_t ext4_mb_prefetch(struct super_block *sb,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 880f383df..fd350889a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -100,7 +100,7 @@ static int ext4_ext_trunc_restart_fn(struct inode *inode, int *dropped)
* i_rwsem. So we can safely drop the i_data_sem here.
*/
BUG_ON(EXT4_JOURNAL(inode) == NULL);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
up_write(&EXT4_I(inode)->i_data_sem);
*dropped = 1;
return 0;
@@ -4313,7 +4313,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
* not a good idea to call discard here directly,
* but otherwise we'd need to call it every free().
*/
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE)
fb_flags = EXT4_FREE_BLOCKS_NO_QUOT_UPDATE;
ext4_free_blocks(handle, inode, NULL, newblock,
@@ -5354,7 +5354,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_FALLOC_RANGE, handle);
down_write(&EXT4_I(inode)->i_data_sem);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
ext4_es_remove_extent(inode, punch_start, EXT_MAX_BLOCKS - punch_start);
ret = ext4_ext_remove_space(inode, punch_start, punch_stop - 1);
@@ -5362,7 +5362,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
up_write(&EXT4_I(inode)->i_data_sem);
goto out_stop;
}
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
ret = ext4_ext_shift_extents(inode, handle, punch_stop,
punch_stop - punch_start, SHIFT_LEFT);
@@ -5494,7 +5494,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
goto out_stop;
down_write(&EXT4_I(inode)->i_data_sem);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
path = ext4_find_extent(inode, offset_lblk, NULL, 0);
if (IS_ERR(path)) {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 0166bb9ca..89cb28da8 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -174,7 +174,7 @@ static int ext4_release_file(struct inode *inode, struct file *filp)
(atomic_read(&inode->i_writecount) == 1) &&
!EXT4_I(inode)->i_reserved_data_blocks) {
down_write(&EXT4_I(inode)->i_data_sem);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
up_write(&EXT4_I(inode)->i_data_sem);
}
if (is_dx(inode) && filp->private_data)
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index a9f371611..d8ca7f64f 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -714,7 +714,7 @@ static int ext4_ind_trunc_restart_fn(handle_t *handle, struct inode *inode,
* i_rwsem. So we can safely drop the i_data_sem here.
*/
BUG_ON(EXT4_JOURNAL(inode) == NULL);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
up_write(&EXT4_I(inode)->i_data_sem);
*dropped = 1;
return 0;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d77323204..53ed4a0ad 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -371,7 +371,7 @@ void ext4_da_update_reserve_space(struct inode *inode,
*/
if ((ei->i_reserved_data_blocks == 0) &&
!inode_is_open_for_write(inode))
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
}
static int __check_block_validity(struct inode *inode, const char *func,
@@ -4014,7 +4014,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
if (stop_block > first_block) {
down_write(&EXT4_I(inode)->i_data_sem);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
ext4_es_remove_extent(inode, first_block,
stop_block - first_block);
@@ -4167,7 +4167,7 @@ int ext4_truncate(struct inode *inode)
down_write(&EXT4_I(inode)->i_data_sem);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
err = ext4_ext_truncate(handle, inode);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0bfe2ce58..2e2bcd22b 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -458,7 +458,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
ext4_reset_inode_seed(inode);
ext4_reset_inode_seed(inode_bl);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
err = ext4_mark_inode_dirty(handle, inode);
if (err < 0) {
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index e6561a09d..0e6beb3b4 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5469,7 +5469,7 @@ ext4_mb_discard_group_preallocations(struct super_block *sb,
*
* FIXME!! Make sure it is valid at all the call sites
*/
-void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
+void ext4_discard_preallocations(struct inode *inode)
{
struct ext4_inode_info *ei = EXT4_I(inode);
struct super_block *sb = inode->i_sb;
@@ -5491,15 +5491,12 @@ void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
mb_debug(sb, "discard preallocation for inode %lu\n",
inode->i_ino);
trace_ext4_discard_preallocations(inode,
- atomic_read(&ei->i_prealloc_active), needed);
-
- if (needed == 0)
- needed = UINT_MAX;
+ atomic_read(&ei->i_prealloc_active), 0);
repeat:
/* first, collect all pa's in the inode */
write_lock(&ei->i_prealloc_lock);
- for (iter = rb_first(&ei->i_prealloc_node); iter && needed;
+ for (iter = rb_first(&ei->i_prealloc_node); iter;
iter = rb_next(iter)) {
pa = rb_entry(iter, struct ext4_prealloc_space,
pa_node.inode_node);
@@ -5523,7 +5520,6 @@ void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
spin_unlock(&pa->pa_lock);
rb_erase(&pa->pa_node.inode_node, &ei->i_prealloc_node);
list_add(&pa->u.pa_tmp_list, &list);
- needed--;
continue;
}
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 18a9e7c47..0abfc104a 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -689,8 +689,8 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
out:
if (*moved_len) {
- ext4_discard_preallocations(orig_inode, 0);
- ext4_discard_preallocations(donor_inode, 0);
+ ext4_discard_preallocations(orig_inode);
+ ext4_discard_preallocations(donor_inode);
}
ext4_free_ext_path(path);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d062383ea..c0cfc3e46 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1518,7 +1518,7 @@ void ext4_clear_inode(struct inode *inode)
ext4_fc_del(inode);
invalidate_inode_buffers(inode);
clear_inode(inode);
- ext4_discard_preallocations(inode, 0);
+ ext4_discard_preallocations(inode);
ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
dquot_drop(inode);
if (EXT4_I(inode)->jinode) {
--
2.30.0
On Wed 03-01-24 18:28:21, Kemeng Shi wrote:
> As 'needed' to trace_ext4_discard_preallocations is always 0 which
> is meaningless. Just remove it.
>
> Signed-off-by: Kemeng Shi <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext4/mballoc.c | 5 ++---
> include/trace/events/ext4.h | 11 ++++-------
> 2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 0e6beb3b4..091a832a8 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -5481,9 +5481,8 @@ void ext4_discard_preallocations(struct inode *inode)
> struct rb_node *iter;
> int err;
>
> - if (!S_ISREG(inode->i_mode)) {
> + if (!S_ISREG(inode->i_mode))
> return;
> - }
>
> if (EXT4_SB(sb)->s_mount_state & EXT4_FC_REPLAY)
> return;
> @@ -5491,7 +5490,7 @@ void ext4_discard_preallocations(struct inode *inode)
> mb_debug(sb, "discard preallocation for inode %lu\n",
> inode->i_ino);
> trace_ext4_discard_preallocations(inode,
> - atomic_read(&ei->i_prealloc_active), 0);
> + atomic_read(&ei->i_prealloc_active));
>
> repeat:
> /* first, collect all pa's in the inode */
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index 65029dfb9..a697f4b77 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -772,15 +772,14 @@ TRACE_EVENT(ext4_mb_release_group_pa,
> );
>
> TRACE_EVENT(ext4_discard_preallocations,
> - TP_PROTO(struct inode *inode, unsigned int len, unsigned int needed),
> + TP_PROTO(struct inode *inode, unsigned int len),
>
> - TP_ARGS(inode, len, needed),
> + TP_ARGS(inode, len),
>
> TP_STRUCT__entry(
> __field( dev_t, dev )
> __field( ino_t, ino )
> __field( unsigned int, len )
> - __field( unsigned int, needed )
>
> ),
>
> @@ -788,13 +787,11 @@ TRACE_EVENT(ext4_discard_preallocations,
> __entry->dev = inode->i_sb->s_dev;
> __entry->ino = inode->i_ino;
> __entry->len = len;
> - __entry->needed = needed;
> ),
>
> - TP_printk("dev %d,%d ino %lu len: %u needed %u",
> + TP_printk("dev %d,%d ino %lu len: %u",
> MAJOR(__entry->dev), MINOR(__entry->dev),
> - (unsigned long) __entry->ino, __entry->len,
> - __entry->needed)
> + (unsigned long) __entry->ino, __entry->len)
> );
>
> TRACE_EVENT(ext4_mb_discard_preallocations,
> --
> 2.30.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 03-01-24 18:28:20, Kemeng Shi wrote:
> The "needed" controls the number of ext4_prealloc_space to discard in
> ext4_discard_preallocations. Function ext4_discard_preallocations is
> supposed to discard all non-used preallocated blocks when "needed"
> is 0 and now ext4_discard_preallocations is always called with "needed"
> = 0. Remove unnecessary parameter "needed" and remove all non-used
> preallocated spaces in ext4_discard_preallocations to simplify the
> code.
>
> Note: If count of non-used preallocated spaces could be more than
> UINT_MAX, there was a memory leak as some non-used preallocated
> spaces are left ununsed and this commit will fix it. Otherwise,
> there is no behavior change.
>
> Signed-off-by: Kemeng Shi <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext4/ext4.h | 2 +-
> fs/ext4/extents.c | 10 +++++-----
> fs/ext4/file.c | 2 +-
> fs/ext4/indirect.c | 2 +-
> fs/ext4/inode.c | 6 +++---
> fs/ext4/ioctl.c | 2 +-
> fs/ext4/mballoc.c | 10 +++-------
> fs/ext4/move_extent.c | 4 ++--
> fs/ext4/super.c | 2 +-
> 9 files changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index fb35cae16..2fd444034 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2905,7 +2905,7 @@ extern int ext4_mb_init(struct super_block *);
> extern void ext4_mb_release(struct super_block *);
> extern ext4_fsblk_t ext4_mb_new_blocks(handle_t *,
> struct ext4_allocation_request *, int *);
> -extern void ext4_discard_preallocations(struct inode *, unsigned int);
> +extern void ext4_discard_preallocations(struct inode *);
> extern int __init ext4_init_mballoc(void);
> extern void ext4_exit_mballoc(void);
> extern ext4_group_t ext4_mb_prefetch(struct super_block *sb,
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 880f383df..fd350889a 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -100,7 +100,7 @@ static int ext4_ext_trunc_restart_fn(struct inode *inode, int *dropped)
> * i_rwsem. So we can safely drop the i_data_sem here.
> */
> BUG_ON(EXT4_JOURNAL(inode) == NULL);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> up_write(&EXT4_I(inode)->i_data_sem);
> *dropped = 1;
> return 0;
> @@ -4313,7 +4313,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> * not a good idea to call discard here directly,
> * but otherwise we'd need to call it every free().
> */
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE)
> fb_flags = EXT4_FREE_BLOCKS_NO_QUOT_UPDATE;
> ext4_free_blocks(handle, inode, NULL, newblock,
> @@ -5354,7 +5354,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
> ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_FALLOC_RANGE, handle);
>
> down_write(&EXT4_I(inode)->i_data_sem);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> ext4_es_remove_extent(inode, punch_start, EXT_MAX_BLOCKS - punch_start);
>
> ret = ext4_ext_remove_space(inode, punch_start, punch_stop - 1);
> @@ -5362,7 +5362,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
> up_write(&EXT4_I(inode)->i_data_sem);
> goto out_stop;
> }
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
>
> ret = ext4_ext_shift_extents(inode, handle, punch_stop,
> punch_stop - punch_start, SHIFT_LEFT);
> @@ -5494,7 +5494,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
> goto out_stop;
>
> down_write(&EXT4_I(inode)->i_data_sem);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
>
> path = ext4_find_extent(inode, offset_lblk, NULL, 0);
> if (IS_ERR(path)) {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 0166bb9ca..89cb28da8 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -174,7 +174,7 @@ static int ext4_release_file(struct inode *inode, struct file *filp)
> (atomic_read(&inode->i_writecount) == 1) &&
> !EXT4_I(inode)->i_reserved_data_blocks) {
> down_write(&EXT4_I(inode)->i_data_sem);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> up_write(&EXT4_I(inode)->i_data_sem);
> }
> if (is_dx(inode) && filp->private_data)
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index a9f371611..d8ca7f64f 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -714,7 +714,7 @@ static int ext4_ind_trunc_restart_fn(handle_t *handle, struct inode *inode,
> * i_rwsem. So we can safely drop the i_data_sem here.
> */
> BUG_ON(EXT4_JOURNAL(inode) == NULL);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> up_write(&EXT4_I(inode)->i_data_sem);
> *dropped = 1;
> return 0;
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d77323204..53ed4a0ad 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -371,7 +371,7 @@ void ext4_da_update_reserve_space(struct inode *inode,
> */
> if ((ei->i_reserved_data_blocks == 0) &&
> !inode_is_open_for_write(inode))
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> }
>
> static int __check_block_validity(struct inode *inode, const char *func,
> @@ -4014,7 +4014,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
> if (stop_block > first_block) {
>
> down_write(&EXT4_I(inode)->i_data_sem);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
>
> ext4_es_remove_extent(inode, first_block,
> stop_block - first_block);
> @@ -4167,7 +4167,7 @@ int ext4_truncate(struct inode *inode)
>
> down_write(&EXT4_I(inode)->i_data_sem);
>
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
>
> if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> err = ext4_ext_truncate(handle, inode);
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 0bfe2ce58..2e2bcd22b 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -458,7 +458,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
> ext4_reset_inode_seed(inode);
> ext4_reset_inode_seed(inode_bl);
>
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
>
> err = ext4_mark_inode_dirty(handle, inode);
> if (err < 0) {
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index e6561a09d..0e6beb3b4 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -5469,7 +5469,7 @@ ext4_mb_discard_group_preallocations(struct super_block *sb,
> *
> * FIXME!! Make sure it is valid at all the call sites
> */
> -void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
> +void ext4_discard_preallocations(struct inode *inode)
> {
> struct ext4_inode_info *ei = EXT4_I(inode);
> struct super_block *sb = inode->i_sb;
> @@ -5491,15 +5491,12 @@ void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
> mb_debug(sb, "discard preallocation for inode %lu\n",
> inode->i_ino);
> trace_ext4_discard_preallocations(inode,
> - atomic_read(&ei->i_prealloc_active), needed);
> -
> - if (needed == 0)
> - needed = UINT_MAX;
> + atomic_read(&ei->i_prealloc_active), 0);
>
> repeat:
> /* first, collect all pa's in the inode */
> write_lock(&ei->i_prealloc_lock);
> - for (iter = rb_first(&ei->i_prealloc_node); iter && needed;
> + for (iter = rb_first(&ei->i_prealloc_node); iter;
> iter = rb_next(iter)) {
> pa = rb_entry(iter, struct ext4_prealloc_space,
> pa_node.inode_node);
> @@ -5523,7 +5520,6 @@ void ext4_discard_preallocations(struct inode *inode, unsigned int needed)
> spin_unlock(&pa->pa_lock);
> rb_erase(&pa->pa_node.inode_node, &ei->i_prealloc_node);
> list_add(&pa->u.pa_tmp_list, &list);
> - needed--;
> continue;
> }
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 18a9e7c47..0abfc104a 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -689,8 +689,8 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
>
> out:
> if (*moved_len) {
> - ext4_discard_preallocations(orig_inode, 0);
> - ext4_discard_preallocations(donor_inode, 0);
> + ext4_discard_preallocations(orig_inode);
> + ext4_discard_preallocations(donor_inode);
> }
>
> ext4_free_ext_path(path);
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d062383ea..c0cfc3e46 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1518,7 +1518,7 @@ void ext4_clear_inode(struct inode *inode)
> ext4_fc_del(inode);
> invalidate_inode_buffers(inode);
> clear_inode(inode);
> - ext4_discard_preallocations(inode, 0);
> + ext4_discard_preallocations(inode);
> ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
> dquot_drop(inode);
> if (EXT4_I(inode)->jinode) {
> --
> 2.30.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed, Jan 03, 2024 at 06:28:14PM +0800, Kemeng Shi wrote:
> Remove unused parameter group in ext4_mb_choose_next_group_*().
Hey Kemeng,
Just a small thing, the parameter we are removing is "ngroups" rather
than "group" so maybe fix that in the commit message.
Other than that looks good, feel free to add
Reviewed-by: Ojaswin Mujoo <[email protected]>
>
> Signed-off-by: Kemeng Shi <[email protected]>
> Reviewed-by: Jan Kara <[email protected]>
> ---
> fs/ext4/mballoc.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 9f9b8dd06..765b62729 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -870,7 +870,7 @@ mb_update_avg_fragment_size(struct super_block *sb, struct ext4_group_info *grp)
> * cr level needs an update.
> */
> static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context *ac,
> - enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
> + enum criteria *new_cr, ext4_group_t *group)
> {
on 1/4/2024 1:57 PM, Ojaswin Mujoo wrote:
> On Wed, Jan 03, 2024 at 06:28:14PM +0800, Kemeng Shi wrote:
>> Remove unused parameter group in ext4_mb_choose_next_group_*().
>
> Hey Kemeng,
>
> Just a small thing, the parameter we are removing is "ngroups" rather
> than "group" so maybe fix that in the commit message.
>
> Other than that looks good, feel free to add
>
Thanks for let me know. I will fix it in next version.
> Reviewed-by: Ojaswin Mujoo <[email protected]>
>>
>> Signed-off-by: Kemeng Shi <[email protected]>
>> Reviewed-by: Jan Kara <[email protected]>
>> ---
>> fs/ext4/mballoc.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 9f9b8dd06..765b62729 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -870,7 +870,7 @@ mb_update_avg_fragment_size(struct super_block *sb, struct ext4_group_info *grp)
>> * cr level needs an update.
>> */
>> static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context *ac,
>> - enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
>> + enum criteria *new_cr, ext4_group_t *group)
>> {
>