Skip to contentSkip to conversation

f2fs: add an ioctl() to explicitly trigger fsck later

PATCH In review Jaegeuk Kim 3 participants 7 messages Nov 29, 2018 – Dec 13, 2018 F2FS file system

lore
Share & tools
lore

Jaegeuk Kim

author maintainer
#0 lore raw

PATCH 1/1 f2fs: add an ioctl() to explicitly trigger fsck later

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
fs/f2fs/f2fs.h +1 -0
index aa500239baf2..7cec897146a3 100644
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */
 
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +7 -0
index ff82350a2c55..ca9bdbb8651b 100644
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;
Signature · 1 line
2.19.0.605.g01d371f741-goog
#1 lore raw
Quoted 14 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
fs/f2fs/f2fs.h +0 -0
index aa500239baf2..7cec897146a3 100644

fs/f2fs/f2fs.h static inline bool __has_cursum_space(struct f2fs_journal *journal, · line 420

Quoted 4 lines
fs/f2fs/f2fs.h +1 -0
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */

Why not add a new ioctl interface for this? F2FS_GOING_DOWN_ prefix implies filesystem will shutdown, IMO, we'd better to keep all sub-interfaces being consistent in f2fs_ioc_shutdown().

Quoted 8 lines
fs/f2fs/f2fs.h +0 -0
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +0 -0
index ff82350a2c55..ca9bdbb8651b 100644

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1974

Quoted 9 lines
fs/f2fs/file.c +6 -0
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;

In large-sized image, it may take long time to trigger full scan during boot, so I'd like to ask how often we set this flag?

Thanks,

Quoted 5 lines
fs/f2fs/file.c +1 -0
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;
author maintainer
#2 lore raw
Quoted 10 lines · Chao Yu
Quoted 9 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
Quoted 13 lines

fs/f2fs/f2fs.h static inline bool __has_cursum_space(struct f2fs_journal *journal, · line 420

Quoted 9 lines
fs/f2fs/f2fs.h +1 -0
index aa500239baf2..7cec897146a3 100644
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */

Why not add a new ioctl interface for this? F2FS_GOING_DOWN_ prefix implies filesystem will shutdown, IMO, we'd better to keep all sub-interfaces being consistent in f2fs_ioc_shutdown().

I'm thinking to use this for QA as device shutdown tests.

Quoted 13 lines
Quoted 13 lines
fs/f2fs/f2fs.h +0 -0
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +1 -0
index ff82350a2c55..ca9bdbb8651b 100644
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
Quoted 8 lines

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1974

Quoted 5 lines
fs/f2fs/file.c +5 -0
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;

In large-sized image, it may take long time to trigger full scan during boot, so I'd like to ask how often we set this flag?

Based on the use of shutdown ioctl, I'll use this for testing purpose, so it won't be used for regular cases.

Quoted 8 lines

Thanks,

Quoted 5 lines
fs/f2fs/file.c +1 -0
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;
#3 lore raw
Quoted 49 lines · Jaegeuk Kim
Quoted 10 lines · Chao Yu
Quoted 9 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
Quoted 13 lines

fs/f2fs/f2fs.h static inline bool __has_cursum_space(struct f2fs_journal *journal, · line 420

Quoted 9 lines
fs/f2fs/f2fs.h +1 -0
index aa500239baf2..7cec897146a3 100644
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */

Why not add a new ioctl interface for this? F2FS_GOING_DOWN_ prefix implies filesystem will shutdown, IMO, we'd better to keep all sub-interfaces being consistent in f2fs_ioc_shutdown().

I'm thinking to use this for QA as device shutdown tests.

Quoted 13 lines
Quoted 13 lines
fs/f2fs/f2fs.h +0 -0
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +1 -0
index ff82350a2c55..ca9bdbb8651b 100644
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
Quoted 8 lines

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1974

Quoted 5 lines
fs/f2fs/file.c +5 -0
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;

In large-sized image, it may take long time to trigger full scan during boot, so I'd like to ask how often we set this flag?

Quoted 2 lines

Based on the use of shutdown ioctl, I'll use this for testing purpose, so it won't be used for regular cases.

How about adding some commit messages or comments on F2FS_GOING_DOWN_NEED_FSCK marco definition to give a bit hint for f2fs developers to know the usage of the interface?

Thanks,

Quoted 12 lines
Quoted 9 lines

Thanks,

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1978

Quoted 5 lines
fs/f2fs/file.c +1 -0
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;

.

author maintainer
#4 lore raw
Quoted 50 lines · Chao Yu
Quoted 49 lines · Jaegeuk Kim
Quoted 10 lines · Chao Yu
Quoted 9 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
Quoted 13 lines

fs/f2fs/f2fs.h static inline bool __has_cursum_space(struct f2fs_journal *journal, · line 420

Quoted 9 lines
fs/f2fs/f2fs.h +1 -0
index aa500239baf2..7cec897146a3 100644
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */

Why not add a new ioctl interface for this? F2FS_GOING_DOWN_ prefix implies filesystem will shutdown, IMO, we'd better to keep all sub-interfaces being consistent in f2fs_ioc_shutdown().

I'm thinking to use this for QA as device shutdown tests.

Quoted 21 lines
Quoted 9 lines
fs/f2fs/f2fs.h +0 -0
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +0 -0
index ff82350a2c55..ca9bdbb8651b 100644

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1974

Quoted 9 lines
fs/f2fs/file.c +6 -0
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;

In large-sized image, it may take long time to trigger full scan during boot, so I'd like to ask how often we set this flag?

Quoted 6 lines
Quoted 2 lines

Based on the use of shutdown ioctl, I'll use this for testing purpose, so it won't be used for regular cases.

How about adding some commit messages or comments on F2FS_GOING_DOWN_NEED_FSCK marco definition to give a bit hint for f2fs developers to know the usage of the interface?

Done. :P

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev

Quoted 15 lines

Thanks,

Quoted 12 lines
Quoted 9 lines

Thanks,

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1978

Quoted 5 lines
fs/f2fs/file.c +1 -0
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;

.

Reviewed-by
#5 lore raw
Quoted 54 lines · Jaegeuk Kim
Quoted 53 lines · Chao Yu
Quoted 51 lines · Jaegeuk Kim
Quoted 10 lines · Chao Yu
Quoted 9 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/f2fs.h | 1 +
 fs/f2fs/file.c | 7 +++++++
 2 files changed, 8 insertions(+)
Quoted 13 lines

fs/f2fs/f2fs.h static inline bool __has_cursum_space(struct f2fs_journal *journal, · line 420

Quoted 9 lines
fs/f2fs/f2fs.h +1 -0
index aa500239baf2..7cec897146a3 100644
@@ -417,6 +417,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
 #define F2FS_GOING_DOWN_METASYNC	0x1	/* going down with metadata */
 #define F2FS_GOING_DOWN_NOSYNC		0x2	/* going down */
 #define F2FS_GOING_DOWN_METAFLUSH	0x3	/* going down with meta flush */
+#define F2FS_GOING_DOWN_NEED_FSCK	0x4	/* going down to trigger fsck */

Why not add a new ioctl interface for this? F2FS_GOING_DOWN_ prefix implies filesystem will shutdown, IMO, we'd better to keep all sub-interfaces being consistent in f2fs_ioc_shutdown().

I'm thinking to use this for QA as device shutdown tests.

Quoted 13 lines
Quoted 13 lines
fs/f2fs/f2fs.h +0 -0
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
fs/f2fs/file.c +1 -0
index ff82350a2c55..ca9bdbb8651b 100644
@@ -1966,6 +1966,13 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		f2fs_stop_checkpoint(sbi, false);
 		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
+	case F2FS_GOING_DOWN_NEED_FSCK:
Quoted 8 lines

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1974

Quoted 5 lines
fs/f2fs/file.c +5 -0
+		set_sbi_flag(sbi, SBI_NEED_FSCK);
+		/* do checkpoint only */
+		ret = f2fs_sync_fs(sb, 1);
+		if (ret)
+			goto out;

In large-sized image, it may take long time to trigger full scan during boot, so I'd like to ask how often we set this flag?

Based on the use of shutdown ioctl, I'll use this for testing purpose, so it won't be used for regular cases.

Quoted 7 lines
Quoted 3 lines

How about adding some commit messages or comments on F2FS_GOING_DOWN_NEED_FSCK marco definition to give a bit hint for f2fs developers to know the usage of the interface?

Done. :P

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev

Looks good to me now. ;)

Reviewed-by Chao Yu <[email protected]>

Thanks,

Quoted 19 lines
Quoted 16 lines

Thanks,

Quoted 12 lines
Quoted 9 lines

Thanks,

fs/f2fs/file.c static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) · line 1978

Quoted 5 lines
fs/f2fs/file.c +1 -0
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;

.

.

#6 lore raw
In reply to Jaegeuk Kim
Quoted 4 lines · Jaegeuk Kim

This adds an option in ioctl(F2FS_IOC_SHUTDOWN) in order to trigger fsck by setting a NEED_FSCK flag.

Signed-off-by Jaegeuk Kim <[email protected]>

I don't expect to use it too often, but it makes sense.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html