When the first error is copied into the on-disk superblock, ext4_update_super() is supposed to start the periodic error reporting timer if it is not already running and err_report_sec is non-zero, as the comment above the code says. However, the check was inverted: if (!es->s_error_count && !sbi->s_err_report_sec) With the default err_report_sec of 86400 the timer is therefore never started when a clean file system records its first error, so the "error count since last fsck" messages are not printed until the file system is remounted. Conversely, if the administrator disabled the report by writing 0 to /sys/fs/ext4//err_report_sec, the first error arms the timer to fire immediately. Drop the stray negation. Also read s_err_report_sec only once, with READ_ONCE(), since it can be changed concurrently via sysfs, so that the value tested is the value used to arm the timer. Fixes: d518215c2719 ("ext4: add sysfs attribute err_report_sec to control s_err_report timer") Signed-off-by: Theodore Ts'o --- fs/ext4/super-test.c | 12 ++++++++++++ fs/ext4/super.c | 6 ++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/ext4/super-test.c b/fs/ext4/super-test.c index 3fd7a4d86f31..abd844c8c29f 100644 --- a/fs/ext4/super-test.c +++ b/fs/ext4/super-test.c @@ -1305,6 +1305,18 @@ static void test_error_recording_and_update_super(struct kunit *test) KUNIT_EXPECT_EQ(test, le64_to_cpu(es->s_kbytes_written), 100ULL); KUNIT_EXPECT_EQ(test, es->s_checksum, ext4_superblock_csum(es)); + /* First recorded error arms the periodic error report timer */ + KUNIT_EXPECT_TRUE(test, timer_pending(&sbi->s_err_report)); + + /* err_report_sec == 0 disables the timer even for a first error */ + timer_delete_sync(&sbi->s_err_report); + es->s_error_count = 0; + sbi->s_err_report_sec = 0; + ext4_save_error_info_test(sb, EIO, 31, 4096, "third_fn", 333); + ext4_update_super_test(sb); + KUNIT_EXPECT_EQ(test, le32_to_cpu(es->s_error_count), 1U); + KUNIT_EXPECT_FALSE(test, timer_pending(&sbi->s_err_report)); + /* Read-only superblock update must not touch s_wtime */ es->s_wtime = cpu_to_le32(123456U); saved_wtime = es->s_wtime; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6cdcae00d569..5f704c43c901 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -6302,6 +6302,7 @@ static void ext4_update_super(struct super_block *sb) struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_super_block *es = sbi->s_es; struct buffer_head *sbh = sbi->s_sbh; + unsigned long err_report_sec; lock_buffer(sbh); /* @@ -6360,9 +6361,10 @@ static void ext4_update_super(struct super_block *sb) * Start the daily error reporting function if it hasn't been * started already and sbi->s_err_report_sec is not zero */ - if (!es->s_error_count && !sbi->s_err_report_sec) + err_report_sec = READ_ONCE(sbi->s_err_report_sec); + if (!es->s_error_count && err_report_sec) mod_timer(&sbi->s_err_report, - jiffies + secs_to_jiffies(sbi->s_err_report_sec)); + jiffies + secs_to_jiffies(err_report_sec)); le32_add_cpu(&es->s_error_count, sbi->s_add_error_count); sbi->s_add_error_count = 0; } -- 2.56.0.542.g3ef89b7020-goog