ext4 日志checkpoint与do_get_write_access并发可能导致buffer dirty被清除没有落盘

【标题描述】ext4 日志checkpoint与do_get_write_access并发可能导致buffer dirty被清除没有落盘
【环境信息】
硬件信息：
NA
软件信息：
OLK
【问题复现步骤】
具体操作步骤
```
1. Apply diff and compile kernel
2. ./test.sh
3. panic后reboot
4. fsck.ext4 -fa /dev/sda
```
出现概率(是否必现，概率性错误)必现
【预期结果】
fsck修复失败
【实际结果】
```
[ 21.854433] assign g_bh, trace dir 2 buffer, add a(1036)
[ 21.867328] mark buffer dirty
[ 21.884442] checkpoint: bh dirty bit should be cleared before flush
[ 21.885903] Do access to bh
[ 23.958696] Kernel panic - not syncing: DONE
[ 23.960037] CPU: 2 PID: 3605 Comm: chmod Not tainted 5.10.0-02336-g5dfff828bc1a-dirty #880
[ 23.962524] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproje4
[ 23.966437] Call Trace:
[ 23.966899] dump_stack+0xad/0xdc
[ 23.967359] panic+0x185/0x4b2
[ 23.967778] jbd2_log_do_checkpoint.cold+0x7f/0xb0
[ 23.968433] __jbd2_log_wait_for_space+0x18b/0x350
[ 23.969087] add_transaction_credits+0x3fc/0x4d0
[ 23.969722] ? _raw_spin_unlock_irqrestore+0x34/0x60
[ 23.970407] ? add_timer+0x1dd/0x360
[ 23.970898] start_this_handle+0x156/0x930
[ 23.971465] ? kmem_cache_alloc+0x638/0xa60
[ 23.972037] jbd2__journal_start+0x124/0x300
[ 23.972629] __ext4_journal_start_sb+0x19f/0x1f0
[ 23.973265] ext4_dirty_inode+0x49/0xb0
[ 23.973815] __mark_inode_dirty+0x72/0x670
[ 23.974378] ext4_setattr+0x7bd/0xf80
[ 23.974871] ? putname+0x6c/0xa0
[ 23.975314] ? filename_lookup+0x12d/0x240
[ 23.975897] notify_change+0x39c/0x710
[ 23.976412] ? chmod_common+0xfc/0x1e0
[ 23.976912] chmod_common+0xfc/0x1e0
[ 23.977396] do_fchmodat+0x6e/0xf0
[ 23.977850] __x64_sys_fchmodat+0x1e/0x30
[ 23.978361] do_syscall_64+0x45/0x70
[ 23.978820] entry_SYSCALL_64_after_hwframe+0x61/0xc6
[ 23.979469] RIP: 0033:0x7fc527f7adb9
[ 23.979936] Code: ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f7 c1 ff fe ff ff 75 38 80 e5 01 75 4b 48 63 ff 89 d2 b0
[ 23.982287] RSP: 002b:00007ffccb1ff4f8 EFLAGS: 00000246 ORIG_RAX: 000000000000010c
[ 23.983244] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007fc527f7adb9
[ 23.984141] RDX: 00000000000001ed RSI: 0000560ff50420f0 RDI: ffffffffffffff9c
[ 23.985028] RBP: 0000560ff5042060 R08: 0000000000000000 R09: 0000000000000000
[ 23.985947] R10: 0000000000000000 R11: 0000000000000246 R12: 0000560ff420a734
[ 23.986836] R13: 0000560ff50420f0 R14: 0000560ff5043320 R15: 0000000000008000
[ 23.987815] Kernel Offset: disabled
[ 23.988282] ---[ end Kernel panic - not syncing: DONE ]---

[root@localhost ~]# fsck.ext4 -fa /dev/sda
/dev/sda: recovering journal
/dev/sda: Entry 'a' in / (2) references inode 1036 found in group 0's unused inodes area.
FIXED.
/dev/sda: Entry 'a' in / (2) has an incorrect filetype (was 1, should be 0).

/dev/sda: UNEXPECTED INCONSISTENCY; RUN fsck MANUALLY.
(i.e., without -a or -p options)
```
【附件信息】
test.sh
```
#!/bin/bash

gcc -o aa a.c

umount /root/temp
mkfs.ext4 -F -E lazy_itable_init=0 -b 1024 /dev/sda 100M
mount /dev/sda /root/temp
# Make /root/temp/a inode is not in the same block with inode '/'(2)
for i in $(seq 1 1024)
do
	touch /root/temp/f_$i
done
sync
sync
umount /root/temp

mount /dev/sda /root/temp
./aa
```
diff
```
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index b4f48266308b..555235442d57 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2056,6 +2056,8 @@ void ext4_insert_dentry(struct inode *inode,
  * space.  It will return -ENOSPC if no space is available, and -EIO
  * and -EEXIST if directory entry already exists.
  */
+struct buffer_head *g_bh;
+int g_bh_dirty;
 static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
 			     struct inode *dir,
 			     struct inode *inode, struct ext4_dir_entry_2 *de,
@@ -2103,6 +2105,14 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
 	err = ext4_handle_dirty_dirblock(handle, dir, bh);
 	if (err)
 		ext4_std_error(dir->i_sb, err);
+
+	smp_rmb();
+	if (!g_bh && !strcmp(current->comm, "aa")) {
+		pr_err("assign g_bh, trace dir %llu buffer, add %s(%llu)\n", dir->i_ino, fname_name(fname), inode->i_ino);
+		g_bh = bh;
+		smp_wmb();
+	}
+
 	return err ? err : err2;
 }
 
diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 646fb512bd1e..7b86fea5f20d 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -97,15 +97,17 @@ static inline bool __cp_buffer_busy(struct journal_head *jh)
  * Called under j-state_lock *only*.  It will be unlocked if we have to wait
  * for a checkpoint to free up some space in the log.
  */
+extern int g_bh_dirty;
 void __jbd2_log_wait_for_space(journal_t *journal)
 __acquires(&journal->j_state_lock)
 __releases(&journal->j_state_lock)
 {
 	int nblocks, space_left;
+	static int entered = 0;
 	/* assert_spin_locked(&journal->j_state_lock); */
 
 	nblocks = journal->j_max_transaction_buffers;
-	while (jbd2_log_space_left(journal) < nblocks) {
+	while (jbd2_log_space_left(journal) < nblocks || (!strcmp(current->comm, "chmod") && g_bh_dirty && !entered)) {
 		write_unlock(&journal->j_state_lock);
 		mutex_lock_io(&journal->j_checkpoint_mutex);
 
@@ -127,7 +129,7 @@ __releases(&journal->j_state_lock)
 		}
 		spin_lock(&journal->j_list_lock);
 		space_left = jbd2_log_space_left(journal);
-		if (space_left < nblocks) {
+		if (space_left < nblocks || (!strcmp(current->comm, "chmod") && g_bh_dirty)) {
 			int chkpt = journal->j_checkpoint_transactions != NULL;
 			tid_t tid = 0;
 
@@ -136,6 +138,7 @@ __releases(&journal->j_state_lock)
 			spin_unlock(&journal->j_list_lock);
 			write_unlock(&journal->j_state_lock);
 			if (chkpt) {
+				entered = 1;
 				jbd2_log_do_checkpoint(journal);
 			} else if (jbd2_cleanup_journal_tail(journal) == 0) {
 				/* We were able to recover space; yay! */
@@ -195,6 +198,8 @@ __flush_batch(journal_t *journal, int *batch_count)
  * The journal should be locked before calling this function.
  * Called with j_checkpoint_mutex held.
  */
+#include <linux/delay.h>
+extern struct buffer_head *g_bh;
 int jbd2_log_do_checkpoint(journal_t *journal)
 {
 	struct journal_head	*jh;
@@ -202,6 +207,7 @@ int jbd2_log_do_checkpoint(journal_t *journal)
 	transaction_t		*transaction;
 	tid_t			this_tid;
 	int			result, batch_count = 0;
+	int need_shutdown = 0;
 
 	jbd_debug(1, "Start checkpoint\n");
 
@@ -237,6 +243,7 @@ int jbd2_log_do_checkpoint(journal_t *journal)
 	    transaction->t_tid != this_tid)
 		goto out;
 
+	smp_rmb();
 	/* checkpoint all of the transaction's buffers */
 	while (transaction->t_checkpoint_list) {
 		jh = transaction->t_checkpoint_list;
@@ -285,6 +292,10 @@ int jbd2_log_do_checkpoint(journal_t *journal)
 			spin_lock(&journal->j_list_lock);
 			goto restart;
 		}
+		if (g_bh == bh) {
+			pr_err("checkpoint: bh dirty bit should be cleared before flush\n");
+			need_shutdown = 1;
+		}
 		if (!buffer_dirty(bh)) {
 			BUFFER_TRACE(bh, "remove from checkpoint");
 			if (__jbd2_journal_remove_checkpoint(jh))
@@ -316,6 +327,10 @@ int jbd2_log_do_checkpoint(journal_t *journal)
 		unlock_and_flush:
 			spin_unlock(&journal->j_list_lock);
 		retry:
+			if (batch_count && need_shutdown) {
+				pr_err("Do access to bh\n");
+				msleep(2000);
+			}
 			if (batch_count)
 				__flush_batch(journal, &batch_count);
 			spin_lock(&journal->j_list_lock);
@@ -357,6 +372,8 @@ int jbd2_log_do_checkpoint(journal_t *journal)
 out:
 	spin_unlock(&journal->j_list_lock);
 	result = jbd2_cleanup_journal_tail(journal);
+	if (need_shutdown)
+		panic("DONE");
 
 	return (result < 0) ? result : 0;
 }
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 6d9cce22106c..4427dfeebeaf 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -224,12 +224,14 @@ static void sub_reserved_credits(journal_t *journal, int blocks)
  * transaction. Returns 1 if we had to wait, j_state_lock is dropped, and
  * caller must retry.
  */
+extern int g_bh_dirty;
 static int add_transaction_credits(journal_t *journal, int blocks,
 				   int rsv_blocks)
 {
 	transaction_t *t = journal->j_running_transaction;
 	int needed;
 	int total = blocks + rsv_blocks;
+	static int entered = 0;
 
 	/*
 	 * If the current transaction is locked down for commit, wait
@@ -284,13 +286,14 @@ static int add_transaction_credits(journal_t *journal, int blocks,
 	 * *before* starting to dirty potentially checkpointed buffers
 	 * in the new transaction.
 	 */
-	if (jbd2_log_space_left(journal) < journal->j_max_transaction_buffers) {
+	if (jbd2_log_space_left(journal) < journal->j_max_transaction_buffers || (!strcmp(current->comm, "chmod") && g_bh_dirty && !entered)) {
+		entered = 1;
 		atomic_sub(total, &t->t_outstanding_credits);
 		read_unlock(&journal->j_state_lock);
 		jbd2_might_wait_for_commit(journal);
 		write_lock(&journal->j_state_lock);
 		if (jbd2_log_space_left(journal) <
-					journal->j_max_transaction_buffers)
+					journal->j_max_transaction_buffers || (!strcmp(current->comm, "chmod") && g_bh_dirty))
 			__jbd2_log_wait_for_space(journal);
 		write_unlock(&journal->j_state_lock);
 		return 1;
@@ -940,6 +943,9 @@ static void jbd2_freeze_jh_data(struct journal_head *jh)
  * part of the transaction, that is).
  *
  */
+#include <linux/delay.h>
+extern struct buffer_head *g_bh;
+
 static int
 do_get_write_access(handle_t *handle, struct journal_head *jh,
 			int force_copy)
@@ -950,6 +956,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	int error;
 	char *frozen_buffer = NULL;
 	unsigned long start_lock, time_lock;
+	int happened = 0;
 
 	journal = transaction->t_journal;
 
@@ -1052,6 +1059,11 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 			 */
 			JBUFFER_TRACE(jh, "Journalling dirty buffer");
 			set_buffer_jbddirty(bh);
+			smp_rmb();
+			if (g_bh_dirty && g_bh == bh) {
+				pr_err("%s: clear bh dirty\n", __func__);
+				happened = 1;
+			}
 		}
 		__jbd2_journal_file_buffer(jh, transaction, BJ_Reserved);
 		spin_unlock(&journal->j_list_lock);
@@ -1060,6 +1072,12 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	}
 	unlock_buffer(bh);
 
+	if (happened) {
+		pr_err("%s: wait checkpoint submit\n", __func__);
+		mdelay(5000);
+		pr_err("%s: checkpoint submit done\n", __func__);
+	}
+
 	/*
 	 * If there is already a copy-out version of this buffer, then we don't
 	 * need to make another one
@@ -2040,8 +2058,14 @@ static void __jbd2_journal_temp_unlink_buffer(struct journal_head *jh)
 	jh->b_jlist = BJ_None;
 	if (transaction && is_journal_aborted(transaction->t_journal))
 		clear_buffer_jbddirty(bh);
-	else if (test_clear_buffer_jbddirty(bh))
+	else if (test_clear_buffer_jbddirty(bh)) {
 		mark_buffer_dirty(bh);	/* Expose it to the VM */
+		smp_rmb();
+		if (g_bh == bh) {
+			pr_err("mark buffer dirty\n");
+			g_bh_dirty = 1;
+		}
+	}
 }
 
 /*
```
a.c
```
#define _GNU_SOURCE             /* See feature_test_macros(7) */
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mount.h>
#include <getopt.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/xattr.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/syscall.h>
#include <errno.h>
#include <string.h>
#include <linux/reboot.h>
#include <arpa/inet.h>
#include <pthread.h>
#include <grp.h>
#include <sys/prctl.h>
#include <linux/fs.h>
#include <signal.h>
#include <sched.h>
#include <sys/wait.h>
#include <sys/mman.h>
#include <linux/perf_event.h>
#include <linux/bpf.h>
#include <sys/sysinfo.h>
#include <sys/epoll.h>
#include <asm/unistd.h>
#include <sys/time.h>
#include <sys/sendfile.h>
#include <stdarg.h>
#include <assert.h>
#include <linux/aio_abi.h>

int main(void)
{
	__u64 sz = 1048576;
	int ret, fd = open("/root/temp", O_RDONLY);
	if (fd < 0) {
		perror("open");
		return -1;
	}
	int new_fd = open("/root/temp/a", O_CREAT | O_RDWR, 0666);
	fsync(new_fd);   // trans commit and mark bh dirty

ret = fork();
	if (ret < 0) {
		perror("fork");
		return -1;
	} else if (ret == 0) {
		system("chmod +x /root/temp/a");
		return 0;
	} else {
		/*printf("wait \'Do access to bh\'\n");
		getchar();
		printf("do_get_write_access\n");*/
		sleep(1);
		creat("/root/temp/b", 0666);  // clear bh dirty
	}
	return 0;
}
```

GVP openEuler/kernel

内容风险标识

评论 (2)

GVPopenEuler/kernel

内容风险标识