diff mbox series

[RFC,v3,09/61] e2fsck: create logs for mult-threads

Message ID 20201118153947.3394530-10-saranyamohan@google.com
State New
Headers show
Series Introduce parallel fsck to e2fsck pass1 | expand

Commit Message

Saranya Muruganandam Nov. 18, 2020, 3:38 p.m. UTC
From: Li Xi <lixi@ddn.com>

When multi-threads are used, different logs should be created
for different threads. Each thread has log files with suffix
of ".$THREAD_INDEX".

And this patch adds f_multithread_logfile test case.

Signed-off-by: Li Xi <lixi@ddn.com>
Signed-off-by: Wang Shilong <wshilong@ddn.com>
Signed-off-by: Saranya Muruganandam <saranyamohan@google.com>
---
 e2fsck/e2fsck.h                      |  6 ++++--
 e2fsck/logfile.c                     | 10 ++++++++-
 e2fsck/pass1.c                       | 23 +++++++++++++++-----
 tests/f_multithread_logfile/expect.1 | 23 ++++++++++++++++++++
 tests/f_multithread_logfile/image.gz |  1 +
 tests/f_multithread_logfile/name     |  1 +
 tests/f_multithread_logfile/script   | 32 ++++++++++++++++++++++++++++
 7 files changed, 88 insertions(+), 8 deletions(-)
 create mode 100644 tests/f_multithread_logfile/expect.1
 create mode 120000 tests/f_multithread_logfile/image.gz
 create mode 100644 tests/f_multithread_logfile/name
 create mode 100644 tests/f_multithread_logfile/script

Comments

Theodore Y. Ts'o Nov. 23, 2020, 11:05 p.m. UTC | #1
On Wed, Nov 18, 2020 at 07:38:55AM -0800, Saranya Muruganandam wrote:
> From: Li Xi <lixi@ddn.com>
> 
> When multi-threads are used, different logs should be created
> for different threads. Each thread has log files with suffix
> of ".$THREAD_INDEX".
> 
> And this patch adds f_multithread_logfile test case.

I'm not convinced this is the best approach; why not add a
(thread-aware) log abstraction?  That way there can be a single
logfile, which is going to be much more user/admin/SRE-friendly.

We also need to add some kind of mutex if we are multi-threading in
e2fsck/message.c, since POSIX guarantees that the dio functions are
thread-safe --- but only on a per function basis.  So if we don't want
the output to the terminal to potentially be scrambled, we need to
take a mutex in print_e2fsck_message() if we are in multi-threading
mode, so that only one thread is printing to the terminal at a time.

(And certainly if we are going to be asking a question of the user, we
*definitely* need to be taking some kind of mutex first!)

	     	     	       	    	 - Ted
diff mbox series

Patch

diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index 1416f15e..5ad0fe93 100644
--- a/e2fsck/e2fsck.h
+++ b/e2fsck/e2fsck.h
@@ -308,8 +308,10 @@  struct e2fsck_struct {
 	/*
 	 * For pass1_check_directory and pass1_get_blocks
 	 */
-	ext2_ino_t stashed_ino;
-	struct ext2_inode *stashed_inode;
+	ext2_ino_t		stashed_ino;
+	struct ext2_inode	*stashed_inode;
+	/* Thread index, if global_ctx is null, this field is unused */
+	int			thread_index;
 
 	/*
 	 * Location of the lost and found directory
diff --git a/e2fsck/logfile.c b/e2fsck/logfile.c
index 63e9a12f..c5505d27 100644
--- a/e2fsck/logfile.c
+++ b/e2fsck/logfile.c
@@ -16,6 +16,7 @@ 
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <assert.h>
 
 #include "e2fsck.h"
 #include <pwd.h>
@@ -291,6 +292,8 @@  static FILE *set_up_log_file(e2fsck_t ctx, const char *key, const char *fn)
 	struct string s, s1, s2;
 	char *s0 = 0, *log_dir = 0, *log_fn = 0;
 	int log_dir_wait = 0;
+	int string_size;
+	char string_index[10];
 
 	s.s = s1.s = s2.s = 0;
 
@@ -307,6 +310,12 @@  static FILE *set_up_log_file(e2fsck_t ctx, const char *key, const char *fn)
 		goto out;
 
 	expand_logfn(ctx, log_fn, &s);
+	if (ctx->global_ctx) {
+		sprintf(string_index, "%d", ctx->thread_index);
+		append_string(&s, ".", 1);
+		append_string(&s, string_index, 0);
+	}
+
 	if ((log_fn[0] == '/') || !log_dir || !log_dir[0])
 		s0 = s.s;
 
@@ -325,7 +334,6 @@  static FILE *set_up_log_file(e2fsck_t ctx, const char *key, const char *fn)
 		append_string(&s2, log_dir, 0);
 		append_string(&s2, "/", 1);
 		append_string(&s2, s.s, 0);
-		printf("%s\n", s2.s);
 	}
 
 	if (s0)
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 10efa0ed..bae47a7f 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -2250,6 +2250,9 @@  static errcode_t e2fsck_pass1_thread_prepare(e2fsck_t global_ctx, e2fsck_t *thre
 	}
 	thread_fs->priv_data = thread_context;
 
+	thread_context->thread_index = 0;
+	set_up_logging(thread_context);
+
 	thread_context->fs = thread_fs;
 	*thread_ctx = thread_context;
 	return 0;
@@ -2262,12 +2265,14 @@  out_context:
 
 static int e2fsck_pass1_thread_join_one(e2fsck_t global_ctx, e2fsck_t thread_ctx)
 {
-	errcode_t	retval;
-	int		flags = global_ctx->flags;
-	ext2_filsys	thread_fs = thread_ctx->fs;
-	ext2_filsys	global_fs = global_ctx->fs;
+	errcode_t	 retval;
+	int		 flags = global_ctx->flags;
+	ext2_filsys	 thread_fs = thread_ctx->fs;
+	ext2_filsys	 global_fs = global_ctx->fs;
+	FILE		*global_logf = global_ctx->logf;
+	FILE		*global_problem_logf = global_ctx->problem_logf;
 #ifdef HAVE_SETJMP_H
-	jmp_buf		old_jmp;
+	jmp_buf		 old_jmp;
 
 	memcpy(old_jmp, global_ctx->abort_loc, sizeof(jmp_buf));
 #endif
@@ -2286,6 +2291,8 @@  static int e2fsck_pass1_thread_join_one(e2fsck_t global_ctx, e2fsck_t thread_ctx
 	}
 	global_fs->priv_data = global_ctx;
 	global_ctx->fs = global_fs;
+	global_ctx->logf = global_logf;
+	global_ctx->problem_logf = global_problem_logf;
 
 	if (thread_ctx->inode_used_map) {
 		retval = e2fsck_pass1_copy_bitmap(global_fs,
@@ -2374,6 +2381,12 @@  static int e2fsck_pass1_thread_join(e2fsck_t global_ctx, e2fsck_t thread_ctx)
 
 	retval = e2fsck_pass1_thread_join_one(global_ctx, thread_ctx);
 	ext2fs_free_mem(&thread_ctx->fs);
+	if (thread_ctx->logf)
+		fclose(thread_ctx->logf);
+	if (thread_ctx->problem_logf) {
+		fputs("</problem_log>\n", thread_ctx->problem_logf);
+		fclose(thread_ctx->problem_logf);
+	}
 	ext2fs_free_mem(&thread_ctx);
 
 	return retval;
diff --git a/tests/f_multithread_logfile/expect.1 b/tests/f_multithread_logfile/expect.1
new file mode 100644
index 00000000..e2b954d0
--- /dev/null
+++ b/tests/f_multithread_logfile/expect.1
@@ -0,0 +1,23 @@ 
+ext2fs_open2: Bad magic number in super-block
+../e2fsck/e2fsck: Superblock invalid, trying backup blocks...
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+Free blocks count wrong for group #0 (7987, counted=7982).
+Fix? yes
+
+Free blocks count wrong (11602, counted=11597).
+Fix? yes
+
+Free inodes count wrong for group #0 (1493, counted=1488).
+Fix? yes
+
+Free inodes count wrong (2997, counted=2992).
+Fix? yes
+
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 16/3008 files (0.0% non-contiguous), 403/12000 blocks
+Exit status is 1
diff --git a/tests/f_multithread_logfile/image.gz b/tests/f_multithread_logfile/image.gz
new file mode 120000
index 00000000..0fd40018
--- /dev/null
+++ b/tests/f_multithread_logfile/image.gz
@@ -0,0 +1 @@ 
+../f_zero_super/image.gz
\ No newline at end of file
diff --git a/tests/f_multithread_logfile/name b/tests/f_multithread_logfile/name
new file mode 100644
index 00000000..faaabc3b
--- /dev/null
+++ b/tests/f_multithread_logfile/name
@@ -0,0 +1 @@ 
+test "e2fsck -m" option works with "-E log_filename="
diff --git a/tests/f_multithread_logfile/script b/tests/f_multithread_logfile/script
new file mode 100644
index 00000000..4f9ca6f8
--- /dev/null
+++ b/tests/f_multithread_logfile/script
@@ -0,0 +1,32 @@ 
+LOG_FNAME="f_multithread_logfile_xxx"
+FSCK_OPT="-fy -m -y -E log_filename=$LOG_FNAME"
+SKIP_VERIFY="true"
+ONE_PASS_ONLY="true"
+SKIP_CLEANUP="true"
+
+rm -f $LOG_FNAME.* $LOG_FNAME
+
+. $cmd_dir/run_e2fsck
+
+rm -f $test_name.ok $test_name.failed
+cmp -s $OUT1 $EXP1
+status1=$?
+
+if [ "$status1" -eq 0 ]; then
+	if [ ! -f $LOG_FNAME -o ! -f $LOG_FNAME.0 ]; then
+		echo "$LOG_FNAME or $LOG_FNAME.0 is not created" > $test_name.failed
+		echo "$test_name: $test_description: failed"
+	else
+		echo "$test_name: $test_description: ok"
+		touch $test_name.ok
+	fi
+else
+	diff $DIFF_OPTS $test_dir/expect.1 \
+		$test_name.1.log >> $test_name.failed
+        echo "$test_name: $test_description: failed"
+fi
+
+unset IMAGE FSCK_OPT SECOND_FSCK_OPT OUT1 OUT2 EXP1 EXP2
+unset SKIP_VERIFY SKIP_CLEANUP SKIP_GUNZIP ONE_PASS_ONLY PREP_CMD
+unset DESCRIPTION SKIP_UNLINK AFTER_CMD PASS_ZERO
+unset LOG_FINAME