Re: [PATCH v2] ext4: move journal state away from orphan list updates
From: sashiko-bot@kernel.org
Date: 2026-09-07 11:23:44
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Concurrent sysfs writes to err_report_sec corrupt the timer list due to unnecessary timer_setup() -- commit 5546001d405559aa5c5103006d23b436861ae874 Author: Jonas Zhou [off-list ref] Subject: ext4: move journal state away from orphan list updates This patch exchanges s_journal and s_ext4_flags with s_err_report_sec and s_li_request in struct ext4_sb_info. This separates common journal and flag readers from orphan-list writes to improve cache line usage and avoid false sharing.
quoted hunk ↗ jump to hunk
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 724a27e8be613..16ef010782331 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h@@ -1641,9 +1641,13 @@ struct ext4_sb_info { struct super_block *s_sb; struct buffer_head *s_mmp_bh; + /* timeout in seconds for s_err_report; 0 disables the timer. */ + unsigned long s_err_report_sec; +
[Severity: High]
This isn't a bug introduced by this patch, but while looking at the movement
of s_err_report_sec, I noticed a pre-existing issue in its sysfs store
handler.
Can concurrent writes to the err_report_sec sysfs file corrupt the timer list?
Looking at err_report_sec_store() in fs/ext4/sysfs.c, it does not hold any
locks. If two threads concurrently write non-zero values when the current
value is 0:
Thread A
if (!sbi->s_err_report_sec && t)
timer_setup(&sbi->s_err_report, print_daily_error_info, 0);
...
mod_timer(&sbi->s_err_report, ...);
Thread B
if (!sbi->s_err_report_sec && t)
timer_setup(&sbi->s_err_report, print_daily_error_info, 0);
...
mod_timer(&sbi->s_err_report, ...);
Thread A arms the timer and puts it in the active timer wheel. Thread B then
calls timer_setup() which zeroes timer->entry.pprev. This makes Thread B's
subsequent mod_timer() re-insert the timer without detaching it, corrupting
the timer wheel's doubly-linked hlist.
Since the timer is already unconditionally initialized during filesystem
mount, and timer_delete_sync() doesn't deinitialize it, is this timer_setup()
call in the sysfs handler unnecessary?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/a33183c1c24ea47a98afdd4756b49365593a2762.1788779199.git.jonaszhou@zhaoxin.com?part=1