You are welcome.  :)

Best Wishes,
Zac ([email protected])

> -----邮件原件-----
> 发件人: Chao Yu [mailto:[email protected]]
> 发送时间: 2019年5月15日 10:10
> 收件人: zhaowuyun; [email protected]
> 抄送: [email protected]; [email protected]
> 主题: Re: [PATCH v2 1/2] mkfs.f2fs: write fill chunk in sparse file for zeroed
> block
> 
> Hi Wuyun,
> 
> Thanks for the test for both sparse file enhancement patches. :)
> 
> On 2019/5/14 20:22, zhaowuyun wrote:
> > 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