Messages in this thread | | | Subject | Re: [PATCH 2/2] quota: fix warning in dqgrab() | From | "yebin (H)" <> | Date | Sat, 3 Jun 2023 10:57:38 +0800 |
| |
On 2023/5/30 18:15, Jan Kara wrote: > On Sat 27-05-23 09:40:18, Ye Bin wrote: >> There's issue as follows when do fault injection: >> WARNING: CPU: 1 PID: 14870 at include/linux/quotaops.h:51 dquot_disable+0x13b7/0x18c0 >> Modules linked in: >> CPU: 1 PID: 14870 Comm: fsconfig Not tainted 6.3.0-next-20230505-00006-g5107a9c821af-dirty #541 >> RIP: 0010:dquot_disable+0x13b7/0x18c0 >> RSP: 0018:ffffc9000acc79e0 EFLAGS: 00010246 >> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88825e41b980 >> RDX: 0000000000000000 RSI: ffff88825e41b980 RDI: 0000000000000002 >> RBP: ffff888179f68000 R08: ffffffff82087ca7 R09: 0000000000000000 >> R10: 0000000000000001 R11: ffffed102f3ed026 R12: ffff888179f68130 >> R13: ffff888179f68110 R14: dffffc0000000000 R15: ffff888179f68118 >> FS: 00007f450a073740(0000) GS:ffff88882fc00000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00007ffe96f2efd8 CR3: 000000025c8ad000 CR4: 00000000000006e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Call Trace: >> <TASK> >> dquot_load_quota_sb+0xd53/0x1060 >> dquot_resume+0x172/0x230 >> ext4_reconfigure+0x1dc6/0x27b0 >> reconfigure_super+0x515/0xa90 >> __x64_sys_fsconfig+0xb19/0xd20 >> do_syscall_64+0x39/0xb0 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> >> Above issue may happens as follows: >> ProcessA ProcessB ProcessC >> sys_fsconfig >> vfs_fsconfig_locked >> reconfigure_super >> ext4_remount >> dquot_suspend -> suspend all type quota >> >> sys_fsconfig >> vfs_fsconfig_locked >> reconfigure_super >> ext4_remount >> dquot_resume >> ret = dquot_load_quota_sb >> add_dquot_ref >> do_open -> open file O_RDWR >> vfs_open >> do_dentry_open >> get_write_access >> atomic_inc_unless_negative(&inode->i_writecount) >> ext4_file_open >> dquot_file_open >> dquot_initialize >> __dquot_initialize >> dqget >> atomic_inc(&dquot->dq_count); >> >> __dquot_initialize >> __dquot_initialize >> dqget >> if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) >> ext4_acquire_dquot >> -> Return error DQ_ACTIVE_B flag isn't set >> dquot_disable >> invalidate_dquots >> if (atomic_read(&dquot->dq_count)) >> dqgrab >> WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) >> -> Trigger warning >> >> In the above scenario, 'dquot->dq_flags' has no DQ_ACTIVE_B is normal when >> dqgrab(). >> So just remove 'WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))' >> in dqgrab(). >> >> Signed-off-by: Ye Bin <yebin10@huawei.com> > Thanks for the patch! Actually rather than deleting the assertion from > dqgrab() (which can be and is used by filesystems) I'd replace the dqgrab() > use in invalidate_dquots() with plain: > > atomic_inc(&dquot->dq_count); > > Honza This is indeed a good idea, I will send a new version. >> --- >> include/linux/quotaops.h | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h >> index 11a4becff3a9..cb5e4c11e503 100644 >> --- a/include/linux/quotaops.h >> +++ b/include/linux/quotaops.h >> @@ -48,7 +48,6 @@ static inline struct dquot *dqgrab(struct dquot *dquot) >> { >> /* Make sure someone else has active reference to dquot */ >> WARN_ON_ONCE(!atomic_read(&dquot->dq_count)); >> - WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)); >> atomic_inc(&dquot->dq_count); >> return dquot; >> } >> -- >> 2.31.1 >>
| |