Hi Chao,

using the same steps, 
make the userdata partition dirty and fastboot-flash userdata.img to see the 
mount is successful or not

to test the patch, confirm that issue is fixed by this patch.
Hope to see it accepted.

Tested-by: zhaowuyun <[email protected]>

Best Wishes,
Zac ([email protected])

> 
> 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 <[email protected]>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> 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



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to