| Message ID | 20220503100307.44303-1-sunjunchao2870@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add assert for inode->i_io_list in inode_io_list_move_locked. | expand |
On Tue 03-05-22 03:03:07, Jchao Sun wrote: > Cause of patch b35250c, inode->i_io_list will not only be protected by > wb->list_lock, but also inode->i_lock. And in that patch, Added some assert > for inode->i_lock in some functions except inode_io_list_move_locked. > Should complete it to describe the semantics more clearly. Modified comment > correspondingly. I'd slightly rephrase the commit message and reference commit properly. Like: Commit b35250c0816c ("writeback: Protect inode->i_io_list with inode->i_lock") made inode->i_io_list not only protected by wb->list_lock but also inode->i_lock. The commit also added some asserts for inode->i_lock but inode_io_list_move_locked() was missed. Add assert there and also update comment describing things protected by inode->i_lock. Also please add Fixes: b35250c0816c ("writeback: Protect inode->i_io_list with inode->i_lock") tag. > Signed-off-by: Jchao Sun <sunjunchao2870@gmail.com> Otherwise a good catch. Thanks. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/fs-writeback.c | 1 + > fs/inode.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > index 591fe9cf1659..5a761b39f36c 100644 > --- a/fs/fs-writeback.c > +++ b/fs/fs-writeback.c > @@ -120,6 +120,7 @@ static bool inode_io_list_move_locked(struct inode *inode, > struct list_head *head) > { > assert_spin_locked(&wb->list_lock); > + assert_spin_locked(&inode->i_lock); > > list_move(&inode->i_io_list, head); > > diff --git a/fs/inode.c b/fs/inode.c > index 9d9b422504d1..bd4da9c5207e 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -27,7 +27,7 @@ > * Inode locking rules: > * > * inode->i_lock protects: > - * inode->i_state, inode->i_hash, __iget() > + * inode->i_state, inode->i_hash, __iget(), inode->i_io_list > * Inode LRU list locks protect: > * inode->i_sb->s_inode_lru, inode->i_lru > * inode->i_sb->s_inode_list_lock protects: > -- > 2.17.1 >
Greeting,
FYI, we noticed the following commit (built with clang-15):
commit: 38923ae236aa4f170d793d6dacbe4189dabb821d ("[PATCH] Add assert for inode->i_io_list in inode_io_list_move_locked.")
url: https://github.com/intel-lab-lkp/linux/commits/Jchao-Sun/Add-assert-for-inode-i_io_list-in-inode_io_list_move_locked/20220503-180501
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 9050ba3a61a4b5bd84c2cde092a100404f814f31
patch link: https://lore.kernel.org/linux-fsdevel/20220503100307.44303-1-sunjunchao2870@gmail.com
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+------------------------------------------+------------+------------+
| | 9050ba3a61 | 38923ae236 |
+------------------------------------------+------------+------------+
| boot_successes | 28 | 0 |
| boot_failures | 0 | 12 |
| kernel_BUG_at_fs/fs-writeback.c | 0 | 12 |
| invalid_opcode:#[##] | 0 | 12 |
| EIP:inode_io_list_move_locked | 0 | 12 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 12 |
+------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <oliver.sang@intel.com>
[ 17.888886][ T1] ------------[ cut here ]------------
[ 17.889286][ T1] kernel BUG at fs/fs-writeback.c:123!
[ 17.889622][ T1] invalid opcode: 0000 [#1]
[ 17.889898][ T1] CPU: 0 PID: 1 Comm: systemd Not tainted 5.18.0-rc5-00007-g38923ae236aa #1 25cc0f213b119f28f3b53dcc3b7d88dd069c3958
[ 17.890642][ T1] EIP: inode_io_list_move_locked+0xf7/0x110
[ 17.891808][ T1] Code: 8e 88 00 00 00 01 48 38 b0 01 eb 09 89 f1 e8 30 f0 ff ff 31 c0 5e 5f 5b 5d 31 c9 31 d2 c3 0f 0b 68 90 0c 5e c2 e8 19 97 4d 00 <
0f> 0b 68 a0 0c 5e c2 e8 0d 97 4d 00 90 90 90 90 90 90 90 90 90 90
[ 17.893017][ T1] EAX: 00000000 EBX: 00000001 ECX: 00000000 EDX: 00000000
[ 17.893450][ T1] ESI: c2f3daf4 EDI: f651f000 EBP: c01b7eec ESP: c01b7ee0
[ 17.893878][ T1] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 EFLAGS: 00010202
[ 17.894339][ T1] CR0: 80050033 CR2: b7b2af50 CR3: 09782000 CR4: 00040690
[ 17.894774][ T1] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 17.895202][ T1] DR6: fffe0ff0 DR7: 00000400
[ 17.895485][ T1] Call Trace:
[ 17.895688][ T1] __mark_inode_dirty+0x198/0x1d0
[ 17.895997][ T1] ? iterate_dir+0x151/0x180
[ 17.896289][ T1] touch_atime+0x165/0x220
[ 17.896558][ T1] iterate_dir+0x151/0x180
[ 17.896827][ T1] ? kernfs_rename_ns+0x320/0x320
[ 17.897130][ T1] __ia32_sys_getdents64+0x57/0xe0
[ 17.897440][ T1] ? filldir+0x2c0/0x2c0
[ 17.897698][ T1] do_int80_syscall_32+0x4f/0x70
[ 17.897996][ T1] entry_INT80_32+0x107/0x107
[ 17.898278][ T1] EIP: 0xb7faba02
[ 17.898495][ T1] Code: 95 01 00 05 25 36 02 00 83 ec 14 8d 80 e8 99 ff ff 50 6a 02 e8 1f ff 00 00 c7 04 24 7f 00 00 00 e8 7e 87 01 00 66 90 90 cd 80 <c3> 8d b6 00 00 00 00 8d bc 27 00 00 00 00 8b 1c 24 c3 8d b6 00 00
[ 17.899672][ T1] EAX: ffffffda EBX: 00000021 ECX: 019220e4 EDX: 00008000
[ 17.900110][ T1] ESI: 019220e4 EDI: fffffebc EBP: 00000000 ESP: bfa83190
[ 17.900541][ T1] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
[ 17.901002][ T1] Modules linked in:
[ 17.901255][ T1] ---[ end trace 0000000000000000 ]---
[ 17.901588][ T1] EIP: inode_io_list_move_locked+0xf7/0x110
[ 17.901949][ T1] Code: 8e 88 00 00 00 01 48 38 b0 01 eb 09 89 f1 e8 30 f0 ff ff 31 c0 5e 5f 5b 5d 31 c9 31 d2 c3 0f 0b 68 90 0c 5e c2 e8 19 97 4d 00 <0f> 0b 68 a0 0c 5e c2 e8 0d 97 4d 00 90 90 90 90 90 90 90 90 90 90
[ 17.903152][ T1] EAX: 00000000 EBX: 00000001 ECX: 00000000 EDX: 00000000
[ 17.903580][ T1] ESI: c2f3daf4 EDI: f651f000 EBP: c01b7eec ESP: c01b7ee0
[ 17.904007][ T1] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 EFLAGS: 00010202
[ 17.904473][ T1] CR0: 80050033 CR2: b7b2af50 CR3: 09782000 CR4: 00040690
[ 17.904898][ T1] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 17.905321][ T1] DR6: fffe0ff0 DR7: 00000400
[ 17.905604][ T1] Kernel panic - not syncing: Fatal exception
[ 17.905973][ T1] Kernel Offset: disabled
To reproduce:
# build kernel
cd linux
cp config-5.18.0-rc5-00007-g38923ae236aa .config
make HOSTCC=clang-15 CC=clang-15 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=clang-15 CC=clang-15 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 591fe9cf1659..5a761b39f36c 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -120,6 +120,7 @@ static bool inode_io_list_move_locked(struct inode *inode, struct list_head *head) { assert_spin_locked(&wb->list_lock); + assert_spin_locked(&inode->i_lock); list_move(&inode->i_io_list, head); diff --git a/fs/inode.c b/fs/inode.c index 9d9b422504d1..bd4da9c5207e 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -27,7 +27,7 @@ * Inode locking rules: * * inode->i_lock protects: - * inode->i_state, inode->i_hash, __iget() + * inode->i_state, inode->i_hash, __iget(), inode->i_io_list * Inode LRU list locks protect: * inode->i_sb->s_inode_lru, inode->i_lru * inode->i_sb->s_inode_list_lock protects:
Cause of patch b35250c, inode->i_io_list will not only be protected by wb->list_lock, but also inode->i_lock. And in that patch, Added some assert for inode->i_lock in some functions except inode_io_list_move_locked. Should complete it to describe the semantics more clearly. Modified comment correspondingly. Signed-off-by: Jchao Sun <sunjunchao2870@gmail.com> --- fs/fs-writeback.c | 1 + fs/inode.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)