on non-preempt kernels for filesystems with large number of groups we
may take a long time (>50 ticks) initializing all the groups.
Signed-off-by: Khazhismel Kumykov <[email protected]>
---
fs/ext4/mballoc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 8ef5f12bbee2..c89f497ccf50 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2490,6 +2490,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
sbi->s_buddy_cache->i_ino = EXT4_BAD_INO;
EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
for (i = 0; i < ngroups; i++) {
+ cond_resched();
desc = ext4_get_group_desc(sb, i, NULL);
if (desc == NULL) {
ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
--
2.21.0.392.gf8f6787159e-goog
On Mon 15-04-19 19:59:34, Khazhismel Kumykov wrote:
> on non-preempt kernels for filesystems with large number of groups we
> may take a long time (>50 ticks) initializing all the groups.
>
> Signed-off-by: Khazhismel Kumykov <[email protected]>
Thanks for the patch. I'm not opposed to this but I'm just wondering: Has
this caused any real issues to you? Or how have you come across this
particular loop? Because I'd think we have more similar loops in ext4
codebase...
Honza
> ---
> fs/ext4/mballoc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 8ef5f12bbee2..c89f497ccf50 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2490,6 +2490,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
> sbi->s_buddy_cache->i_ino = EXT4_BAD_INO;
> EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
> for (i = 0; i < ngroups; i++) {
> + cond_resched();
> desc = ext4_get_group_desc(sb, i, NULL);
> if (desc == NULL) {
> ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
> --
> 2.21.0.392.gf8f6787159e-goog
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu, Apr 18, 2019 at 4:53 AM Jan Kara <[email protected]> wrote:
>
> On Mon 15-04-19 19:59:34, Khazhismel Kumykov wrote:
> > on non-preempt kernels for filesystems with large number of groups we
> > may take a long time (>50 ticks) initializing all the groups.
> >
> > Signed-off-by: Khazhismel Kumykov <[email protected]>
>
> Thanks for the patch. I'm not opposed to this but I'm just wondering: Has
> this caused any real issues to you? Or how have you come across this
> particular loop? Because I'd think we have more similar loops in ext4
> codebase...
>
> Honza
>
We have some instrumentation to warn for these longer periods without
scheduling, which admittedly does trigger relatively frequently from
various places. This particular loop just so happens to be during
system startup where it caught my eye.
These functions may take a long time looping over many groups, which
may cause issues for non-preempt kernels.
ext4_mb_init_backend()
ext4_setup_system_zone()
ext4_mb_release()
Signed-off-by: Khazhismel Kumykov <[email protected]>
---
v2:
- a few other places that in testing showed to be slow
fs/ext4/block_validity.c | 1 +
fs/ext4/mballoc.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
index 913061c0de1b..16134469ea3c 100644
--- a/fs/ext4/block_validity.c
+++ b/fs/ext4/block_validity.c
@@ -155,6 +155,7 @@ int ext4_setup_system_zone(struct super_block *sb)
return 0;
for (i=0; i < ngroups; i++) {
+ cond_resched();
if (ext4_bg_has_super(sb, i) &&
((i < 5) || ((i % flex_size) == 0)))
add_system_zone(sbi, ext4_group_first_block_no(sb, i),
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 8ef5f12bbee2..99ba720dbb7a 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2490,6 +2490,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
sbi->s_buddy_cache->i_ino = EXT4_BAD_INO;
EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
for (i = 0; i < ngroups; i++) {
+ cond_resched();
desc = ext4_get_group_desc(sb, i, NULL);
if (desc == NULL) {
ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
@@ -2705,6 +2706,7 @@ int ext4_mb_release(struct super_block *sb)
if (sbi->s_group_info) {
for (i = 0; i < ngroups; i++) {
+ cond_resched();
grinfo = ext4_get_group_info(sb, i);
#ifdef DOUBLE_CHECK
kfree(grinfo->bb_bitmap);
--
2.21.0.593.g511ec345e18-goog
On Apr 23, 2019, at 11:13 PM, Khazhismel Kumykov <[email protected]> wrote:
>
> These functions may take a long time looping over many groups, which
> may cause issues for non-preempt kernels.
> ext4_mb_init_backend()
> ext4_setup_system_zone()
> ext4_mb_release()
>
> Signed-off-by: Khazhismel Kumykov <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
> ---
> v2:
> - a few other places that in testing showed to be slow
>
> fs/ext4/block_validity.c | 1 +
> fs/ext4/mballoc.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
> index 913061c0de1b..16134469ea3c 100644
> --- a/fs/ext4/block_validity.c
> +++ b/fs/ext4/block_validity.c
> @@ -155,6 +155,7 @@ int ext4_setup_system_zone(struct super_block *sb)
> return 0;
>
> for (i=0; i < ngroups; i++) {
> + cond_resched();
> if (ext4_bg_has_super(sb, i) &&
> ((i < 5) || ((i % flex_size) == 0)))
> add_system_zone(sbi, ext4_group_first_block_no(sb, i),
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 8ef5f12bbee2..99ba720dbb7a 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2490,6 +2490,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
> sbi->s_buddy_cache->i_ino = EXT4_BAD_INO;
> EXT4_I(sbi->s_buddy_cache)->i_disksize = 0;
> for (i = 0; i < ngroups; i++) {
> + cond_resched();
> desc = ext4_get_group_desc(sb, i, NULL);
> if (desc == NULL) {
> ext4_msg(sb, KERN_ERR, "can't read descriptor %u", i);
> @@ -2705,6 +2706,7 @@ int ext4_mb_release(struct super_block *sb)
>
> if (sbi->s_group_info) {
> for (i = 0; i < ngroups; i++) {
> + cond_resched();
> grinfo = ext4_get_group_info(sb, i);
> #ifdef DOUBLE_CHECK
> kfree(grinfo->bb_bitmap);
> --
> 2.21.0.593.g511ec345e18-goog
>
Cheers, Andreas
On Tue, Apr 23, 2019 at 02:13:29PM -0700, Khazhismel Kumykov wrote:
> These functions may take a long time looping over many groups, which
> may cause issues for non-preempt kernels.
> ext4_mb_init_backend()
> ext4_setup_system_zone()
> ext4_mb_release()
>
> Signed-off-by: Khazhismel Kumykov <[email protected]>
Thanks, applied.
- Ted