Re: [PATCH -next v2 2/6] ext4: introduce last_check_time record previous check time
From: Jan Kara <jack@suse.cz>
Date: 2021-10-12 08:47:32
Also in:
lkml
On Fri 08-10-21 10:38:31, yebin wrote:
On 2021/10/8 9:56, yebin wrote:quoted
On 2021/10/7 20:31, Jan Kara wrote:quoted
On Sat 11-09-21 17:00:55, Ye Bin wrote:quoted
kmmpd: ... diff = jiffies - last_update_time; if (diff > mmp_check_interval * HZ) { ... As "mmp_check_interval = 2 * mmp_update_interval", 'diff' always little than 'mmp_update_interval', so there will never trigger detection. Introduce last_check_time record previous check time. Signed-off-by: Ye Bin <redacted>I think the check is there only for the case where write_mmp_block() + sleep took longer than mmp_check_interval. I agree that should rarely happen but on a really busy system it is possible and in that case we would miss updating mmp block for too long and so another node could have started using the filesystem. I actually don't see a reason why kmmpd should be checking the block each mmp_check_interval as you do - mmp_check_interval is just for ext4_multi_mount_protect() to know how long it should wait before considering mmp block stale... Am I missing something? HonzaI'm sorry, I didn't understand the detection mechanism here before. Now I understand the detection mechanism here. As you said, it's just an abnormal protection. There's really no problem.Yeah, i did test as following steps hostA hostB mount ext4_multi_mount_protect -> seq == EXT4_MMP_SEQ_CLEAN delay 5s after label "skip" so hostB will see seq is EXT4_MMP_SEQ_CLEAN mount ext4_multi_mount_protect -> seq == EXT4_MMP_SEQ_CLEAN run kmmpd run kmmpd Actually，in this situation kmmpd will not detect confliction. In ext4_multi_mount_protect function we write mmp data first and wait 'wait_time * HZ' seconds, read mmp data do check. Most of the time, If 'wait_time' is zero, it can pass check.
But how can be wait_time zero? As far as I'm reading the code, wait_time must be at least EXT4_MMP_MIN_CHECK_INTERVAL... Honza -- Jan Kara [off-list ref] SUSE Labs, CR