[PATCH v2 1/2] mkfs.f2fs: write fill chunk in sparse file for zeroed block

* [PATCH v2 1/2] mkfs.f2fs: write fill chunk in sparse file for zeroed block
@ 2019-05-05  7:45 Chao Yu
  2019-05-14 12:16 ` 答复: " zhaowuyun via Linux-f2fs-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2019-05-05  7:45 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: jaegeuk, zhaowuyun

As zhaowuyun reported:

we met one problem of f2fs, and found one issue of make_f2fs, so I write
this email to search for your help to confirm this issue.

The issue was found on one of Android projects. We use f2fs as the filesystem
of userdata, and make sparse userdata.img using following command, which
invoked in script mkf2fsuserimg.sh
make_f2fs -S $SIZE -f -O encrypt -O quota -O verity $MKFS_OPTS $OUTPUT_FILE

use fastboot to flash this userdata.img to device, and it encountered f2fs
problem and leading to the mount fail of data partition.

we can make this issue 100% persent reproduced by making the data partition
dirty before flashing userdata.img.

suspect that issue is caused by the dirty data in the data partition.
so we checked that source code of make_f2fs in f2fs-tool, found that when
making f2fs, it use dev_fill to do some process:

...

we change code to the following, and the issue is gone.

if (c.sparse_mode)
       return dev_write(buf, offset, len);

Chao Yu:
>
> After checking the codes, IIUC, I guess the problem here is, unlike
> img2simg, mkfs.f2fs won't record zeroed block in sparse image, so
> during transforming to normal image, some critical region like
> NAT/SIT/CP.payload area weren't be zeroed correctly, later kernel may
> load obsoleting data from those region.
>
> Also, The way you provide will obviously increase the size of sparse
> file, since with it we need to write all zeroed blocks of
> NAT/SIT/CP.payload to sparse file, it's not needed.
>
> Not sure, maybe we should use sparse_file_add_fill() to record zeroed
> blocks, so that this will make formatted image more like img2simged one.

Jaegeuk:
> We have to call sparse_file_add_fill() for dev_fill().

This patch fixes to support writing fill chunk sparse file for those
zeroed blocks in mkfs.f2fs.

Reported-by: zhaowuyun <zhaowuyun@wingtech.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v2:
- don't return -EEXIST if block[x] has non-zeroed data.
 lib/libf2fs_io.c | 84 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 69 insertions(+), 15 deletions(-)

diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index f848510..4d0ea0d 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -36,6 +36,7 @@ struct f2fs_configuration c;
 struct sparse_file *f2fs_sparse_file;
 static char **blocks;
 u_int64_t blocks_count;
+static char *zeroed_block;
 #endif
 
 static int __get_device_fd(__u64 *offset)
@@ -103,6 +104,8 @@ static int sparse_write_blk(__u64 block, int count, const void *buf)
 
 	for (i = 0; i < count; ++i) {
 		cur_block = block + i;
+		if (blocks[cur_block] == zeroed_block)
+			blocks[cur_block] = NULL;
 		if (!blocks[cur_block]) {
 			blocks[cur_block] = calloc(1, F2FS_BLKSIZE);
 			if (!blocks[cur_block])
@@ -114,6 +117,20 @@ static int sparse_write_blk(__u64 block, int count, const void *buf)
 	return 0;
 }
 
+static int sparse_write_zeroed_blk(__u64 block, int count)
+{
+	int i;
+	__u64 cur_block;
+
+	for (i = 0; i < count; ++i) {
+		cur_block = block + i;
+		if (blocks[cur_block])
+			continue;
+		blocks[cur_block] = zeroed_block;
+	}
+	return 0;
+}
+
 #ifdef SPARSE_CALLBACK_USES_SIZE_T
 static int sparse_import_segment(void *UNUSED(priv), const void *data,
 		size_t len, unsigned int block, unsigned int nr_blocks)
@@ -129,11 +146,17 @@ static int sparse_import_segment(void *UNUSED(priv), const void *data, int len,
 	return sparse_write_blk(block, nr_blocks, data);
 }
 
-static int sparse_merge_blocks(uint64_t start, uint64_t num)
+static int sparse_merge_blocks(uint64_t start, uint64_t num, int zero)
 {
 	char *buf;
 	uint64_t i;
 
+	if (zero) {
+		blocks[start] = NULL;
+		return sparse_file_add_fill(f2fs_sparse_file, 0x0,
+					F2FS_BLKSIZE * num, start);
+	}
+
 	buf = calloc(num, F2FS_BLKSIZE);
 	if (!buf) {
 		fprintf(stderr, "failed to alloc %llu\n",
@@ -156,6 +179,7 @@ static int sparse_merge_blocks(uint64_t start, uint64_t num)
 #else
 static int sparse_read_blk(__u64 block, int count, void *buf) { return 0; }
 static int sparse_write_blk(__u64 block, int count, const void *buf) { return 0; }
+static int sparse_write_zeroed_blk(__u64 block, int count) { return 0; }
 #endif
 
 int dev_read(void *buf, __u64 offset, size_t len)
@@ -235,7 +259,8 @@ int dev_fill(void *buf, __u64 offset, size_t len)
 	int fd;
 
 	if (c.sparse_mode)
-		return 0;
+		return sparse_write_zeroed_blk(offset / F2FS_BLKSIZE,
+						len / F2FS_BLKSIZE);
 
 	fd = __get_device_fd(&offset);
 	if (fd < 0)
@@ -307,6 +332,12 @@ int f2fs_init_sparse_file(void)
 		return -1;
 	}
 
+	zeroed_block = calloc(1, F2FS_BLKSIZE);
+	if (!zeroed_block) {
+		MSG(0, "\tError: Calloc Failed for zeroed block!!!\n");
+		return -1;
+	}
+
 	return sparse_file_foreach_chunk(f2fs_sparse_file, true, false,
 				sparse_import_segment, NULL);
 #else
@@ -315,7 +346,8 @@ int f2fs_init_sparse_file(void)
 #endif
 }
 
-#define MAX_CHUNK_SIZE (1 * 1024 * 1024 * 1024ULL)
+#define MAX_CHUNK_SIZE		(1 * 1024 * 1024 * 1024ULL)
+#define MAX_CHUNK_COUNT		(MAX_CHUNK_SIZE / F2FS_BLKSIZE)
 int f2fs_finalize_device(void)
 {
 	int i;
@@ -336,24 +368,44 @@ int f2fs_finalize_device(void)
 		}
 
 		for (j = 0; j < blocks_count; ++j) {
-			if (!blocks[j] && chunk_start != -1) {
-				ret = sparse_merge_blocks(chunk_start,
-							j - chunk_start);
-				chunk_start = -1;
-			} else if (blocks[j] && chunk_start == -1) {
-				chunk_start = j;
-			} else if (blocks[j] && (chunk_start != -1) &&
-				 (j + 1 - chunk_start >=
-					(MAX_CHUNK_SIZE / F2FS_BLKSIZE))) {
+			if (chunk_start != -1) {
+				if (j - chunk_start >= MAX_CHUNK_COUNT) {
+					ret = sparse_merge_blocks(chunk_start,
+							j - chunk_start, 0);
+					ASSERT(!ret);
+					chunk_start = -1;
+				}
+			}
+
+			if (chunk_start == -1) {
+				if (!blocks[j])
+					continue;
+
+				if (blocks[j] == zeroed_block) {
+					ret = sparse_merge_blocks(j, 1, 1);
+					ASSERT(!ret);
+				} else {
+					chunk_start = j;
+				}
+			} else {
+				if (blocks[j] && blocks[j] != zeroed_block)
+					continue;
+
 				ret = sparse_merge_blocks(chunk_start,
-							  j + 1 - chunk_start);
+						j - chunk_start, 0);
+				ASSERT(!ret);
+
+				if (blocks[j] == zeroed_block) {
+					ret = sparse_merge_blocks(j, 1, 1);
+					ASSERT(!ret);
+				}
+
 				chunk_start = -1;
 			}
-			ASSERT(!ret);
 		}
 		if (chunk_start != -1) {
 			ret = sparse_merge_blocks(chunk_start,
-						blocks_count - chunk_start);
+						blocks_count - chunk_start, 0);
 			ASSERT(!ret);
 		}
 
@@ -365,6 +417,8 @@ int f2fs_finalize_device(void)
 			free(blocks[j]);
 		free(blocks);
 		blocks = NULL;
+		free(zeroed_block);
+		zeroed_block = NULL;
 		f2fs_sparse_file = NULL;
 	}
 #endif
-- 
2.18.0.rc1

^ permalink raw reply related	[flat|nested] 3+ messages in thread