| Message ID | 20210817225654.30487-2-shaoyi@amazon.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | ext4: fix lazy initialization next schedule time computation in more granular unit | expand |
On Tue 17-08-21 22:56:54, Shaoying Xu wrote: > Ext4 file system has default lazy inode table initialization setup once > it is mounted. However, it has issue on computing the next schedule time > that makes the timeout same amount in jiffies but different real time in > secs if with various HZ values. Therefore, fix by measuring the current > time in a more granular unit nanoseconds and make the next schedule time > independent of the HZ value. > > Fixes: bfff68738f1c ("ext4: add support for lazy inode table initialization") > Signed-off-by: Shaoying Xu <shaoyi@amazon.com> > Cc: stable@vger.kernel.org Thanks for the patch. It seems to have fallen through the cracks. It looks good just some nits: The timeout will be still dependent on the HZ value because we use jiffie-granular timer. But yes, I guess it is unnecessary to make the imprecision 10x worse when we know we are likely dealing with small numbers. > @@ -3460,14 +3460,13 @@ static int ext4_run_li_request(struct ext4_li_request *elr) > ret = 1; > > if (!ret) { Please add a comment here so that we don't forget. Like: /* Use ns-granular time as init can be really fast */ With this feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> > - timeout = jiffies; > + start_time = ktime_get_real_ns(); > ret = ext4_init_inode_table(sb, group, > elr->lr_timeout ? 0 : 1); > trace_ext4_lazy_itable_init(sb, group); > if (elr->lr_timeout == 0) { > - timeout = (jiffies - timeout) * > - EXT4_SB(elr->lr_super)->s_li_wait_mult; > - elr->lr_timeout = timeout; > + elr->lr_timeout = nsecs_to_jiffies((ktime_get_real_ns() - start_time) * > + EXT4_SB(elr->lr_super)->s_li_wait_mult); > } > elr->lr_next_sched = jiffies + elr->lr_timeout; > elr->lr_next_group = group + 1; Honza
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index dfa09a277b56..399cbe9c14a8 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3421,9 +3421,9 @@ static int ext4_run_li_request(struct ext4_li_request *elr) struct super_block *sb = elr->lr_super; ext4_group_t ngroups = EXT4_SB(sb)->s_groups_count; ext4_group_t group = elr->lr_next_group; - unsigned long timeout = 0; unsigned int prefetch_ios = 0; int ret = 0; + u64 start_time; if (elr->lr_mode == EXT4_LI_MODE_PREFETCH_BBITMAP) { elr->lr_next_group = ext4_mb_prefetch(sb, group, @@ -3460,14 +3460,13 @@ static int ext4_run_li_request(struct ext4_li_request *elr) ret = 1; if (!ret) { - timeout = jiffies; + start_time = ktime_get_real_ns(); ret = ext4_init_inode_table(sb, group, elr->lr_timeout ? 0 : 1); trace_ext4_lazy_itable_init(sb, group); if (elr->lr_timeout == 0) { - timeout = (jiffies - timeout) * - EXT4_SB(elr->lr_super)->s_li_wait_mult; - elr->lr_timeout = timeout; + elr->lr_timeout = nsecs_to_jiffies((ktime_get_real_ns() - start_time) * + EXT4_SB(elr->lr_super)->s_li_wait_mult); } elr->lr_next_sched = jiffies + elr->lr_timeout; elr->lr_next_group = group + 1;
Ext4 file system has default lazy inode table initialization setup once it is mounted. However, it has issue on computing the next schedule time that makes the timeout same amount in jiffies but different real time in secs if with various HZ values. Therefore, fix by measuring the current time in a more granular unit nanoseconds and make the next schedule time independent of the HZ value. Fixes: bfff68738f1c ("ext4: add support for lazy inode table initialization") Signed-off-by: Shaoying Xu <shaoyi@amazon.com> Cc: stable@vger.kernel.org --- fs/ext4/super.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)