| Message ID | 1730354393-19672-2-git-send-email-zhiguo.niu@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev,1/2] f2fs: remove redundant atomic file check in defragment | expand |
On 2024/10/31 13:59, Zhiguo Niu wrote: > If user give a file size as "length" parameter for fiemap > operations, but this size is non-block size aligned, > it will show 2 segments fiemap results even this whole file > is contiguous on disk, such as the following results, please > note that this f2fs_io has been modified for testing. > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > Fiemap: offset = 0 len = 19304 > logical addr. physical addr. length flags > 0 0000000000000000 0000000020baa000 0000000000004000 00001000 > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > > after this patch: > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > Fiemap: offset = 0 len = 19304 > logical addr. physical addr. length flags > 0 0000000000000000 00000000315f3000 0000000000005000 00001000 Why is FIEMAP_EXTENT_LAST missing in #0 extent? As we can see it in #1 extent before your change. 1 0000000000004000 0000000020bae000 0000000000001000 00001001 Thanks, > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > f2fs_io has been modified for testing, the length for fiemap is > real file size, not block number > --- > fs/f2fs/data.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 90fa8ab..8c9bb42 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > goto out; > } > > - if (bytes_to_blks(inode, len) == 0) > - len = blks_to_bytes(inode, 1); > + if (len & (blks_to_bytes(inode, 1) - 1)) > + len = round_up(len, blks_to_bytes(inode, 1)); > > start_blk = bytes_to_blks(inode, start); > last_blk = bytes_to_blks(inode, start + len - 1);
Chao Yu <chao@kernel.org> 于2024年11月1日周五 10:36写道： > > On 2024/10/31 13:59, Zhiguo Niu wrote: > > If user give a file size as "length" parameter for fiemap > > operations, but this size is non-block size aligned, > > it will show 2 segments fiemap results even this whole file > > is contiguous on disk, such as the following results, please > > note that this f2fs_io has been modified for testing. > > > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > Fiemap: offset = 0 len = 19304 > > logical addr. physical addr. length flags > > 0 0000000000000000 0000000020baa000 0000000000004000 00001000 > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > > > > after this patch: > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > Fiemap: offset = 0 len = 19304 > > logical addr. physical addr. length flags > > 0 0000000000000000 00000000315f3000 0000000000005000 00001000 > > Why is FIEMAP_EXTENT_LAST missing in #0 extent? As we can see it > in #1 extent before your change. Hi Chao, for normal fiemap, we just can tag FIEMAP_EXTENT_LAST in the following: /* HOLE */ if (!compr_cluster && !(map.m_flags & F2FS_MAP_FLAGS)) { start_blk = next_pgofs; if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode, max_inode_blocks(inode))) goto prep_next; flags |= FIEMAP_EXTENT_LAST; } but after this patch, if file length = 19304, blk len =5(page size=4096), and f2fs_map_blocks will run once if the file is contiguous on disk, and will tag F2FS_MAP_FLAGS to map.m_flags, so the following case will not run. then if (size) { flags |= FIEMAP_EXTENT_MERGED; if (IS_ENCRYPTED(inode)) flags |= FIEMAP_EXTENT_DATA_ENCRYPTED; ret = fiemap_fill_next_extent(fieinfo, logical, phys, size, flags); trace_f2fs_fiemap(inode, logical, phys, size, flags, ret); if (ret) goto out; size = 0; } fiemap_fill_next_extent will return 1, then go out , this fiemap flow finishes. > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 but back to this case, because next: memset(&map, 0, sizeof(map)); map.m_lblk = start_blk; map.m_len = bytes_to_blks(inode, len); map.m_next_pgofs = &next_pgofs; map.m_seg_type = NO_CHECK_TYPE; map.m_len will not reduce the part that has beed maped even map.m_lbk has changed, for example, if file length = 19304, before patch, map.m_len=4, after once f2fs_map_block, map.m_lbk=4, map.m_len still =4, so there will be "HOLE" case in the follow-up f2fs_map_block so it will loop until start_blk reaches the maxbytes. /* HOLE */ if (!compr_cluster && !(map.m_flags & F2FS_MAP_FLAGS)) { start_blk = next_pgofs; if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode, max_inode_blocks(inode))) goto prep_next; flags |= FIEMAP_EXTENT_LAST; } there is another case before this patch, file size = 5 blocks, if we pass the len not greater than or equal to 5， it will not tag FIEMAP_EXTENT_LAST # ./f2fs_io_new fiemap 0 4 file.apk Fiemap: offset = 0 len = 4 logical addr. physical addr. length flags 0 0000000000000000 000000070e08f000 0000000000004000 00001008 but if len equal or greater than 5, it will tag FIEMAP_EXTENT_LAST # ./f2fs_io_new fiemap 0 5 file.apk Fiemap: offset = 0 len = 5 logical addr. physical addr. length flags 0 0000000000000000 000000070e08f000 0000000000004000 00001008 1 0000000000004000 000000070e090000 0000000000001000 00001001 thanks! > > Thanks, > > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > f2fs_io has been modified for testing, the length for fiemap is > > real file size, not block number > > --- > > fs/f2fs/data.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index 90fa8ab..8c9bb42 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > > goto out; > > } > > > > - if (bytes_to_blks(inode, len) == 0) > > - len = blks_to_bytes(inode, 1); > > + if (len & (blks_to_bytes(inode, 1) - 1)) > > + len = round_up(len, blks_to_bytes(inode, 1)); > > > > start_blk = bytes_to_blks(inode, start); > > last_blk = bytes_to_blks(inode, start + len - 1); >
Zhiguo Niu <niuzhiguo84@gmail.com> 于2024年11月1日周五 11:27写道： > > Chao Yu <chao@kernel.org> 于2024年11月1日周五 10:36写道： > > > > On 2024/10/31 13:59, Zhiguo Niu wrote: > > > If user give a file size as "length" parameter for fiemap > > > operations, but this size is non-block size aligned, > > > it will show 2 segments fiemap results even this whole file > > > is contiguous on disk, such as the following results, please > > > note that this f2fs_io has been modified for testing. > > > > > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > > Fiemap: offset = 0 len = 19304 > > > logical addr. physical addr. length flags > > > 0 0000000000000000 0000000020baa000 0000000000004000 00001000 > > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > > > > > > after this patch: > > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > > Fiemap: offset = 0 len = 19304 > > > logical addr. physical addr. length flags > > > 0 0000000000000000 00000000315f3000 0000000000005000 00001000 > > > > Why is FIEMAP_EXTENT_LAST missing in #0 extent? As we can see it > > in #1 extent before your change. > Hi Chao, > for normal fiemap, we just can tag FIEMAP_EXTENT_LAST in the following: > > /* HOLE */ > if (!compr_cluster && !(map.m_flags & F2FS_MAP_FLAGS)) { > start_blk = next_pgofs; > > if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode, > max_inode_blocks(inode))) > goto prep_next; > > flags |= FIEMAP_EXTENT_LAST; > } > but after this patch, if file length = 19304, blk len =5(page size=4096), > and f2fs_map_blocks will run once if the file is contiguous on disk, > and will tag > F2FS_MAP_FLAGS to map.m_flags, so the following case will not run. > then > > if (size) { > flags |= FIEMAP_EXTENT_MERGED; > if (IS_ENCRYPTED(inode)) > flags |= FIEMAP_EXTENT_DATA_ENCRYPTED; > > ret = fiemap_fill_next_extent(fieinfo, logical, > phys, size, flags); > trace_f2fs_fiemap(inode, logical, phys, size, flags, ret); > if (ret) > goto out; > size = 0; > } > fiemap_fill_next_extent will return 1, then go out , this fiemap flow finishes. > > > > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > but back to this case, because > > next: > memset(&map, 0, sizeof(map)); > map.m_lblk = start_blk; > map.m_len = bytes_to_blks(inode, len); > map.m_next_pgofs = &next_pgofs; > map.m_seg_type = NO_CHECK_TYPE; > > map.m_len will not reduce the part that has beed maped even map.m_lbk > has changed, > for example, if file length = 19304, before patch, map.m_len=4, > after once f2fs_map_block, map.m_lbk=4, map.m_len still =4, so > there will be "HOLE" case in the follow-up f2fs_map_block > so it will loop until start_blk reaches the maxbytes. > /* HOLE */ > if (!compr_cluster && !(map.m_flags & F2FS_MAP_FLAGS)) { > start_blk = next_pgofs; > > if (blks_to_bytes(inode, start_blk) < blks_to_bytes(inode, > max_inode_blocks(inode))) > goto prep_next; > > flags |= FIEMAP_EXTENT_LAST; > } > > there is another case before this patch, file size = 5 blocks, if we > pass the len not > greater than or equal to 5， it will not tag FIEMAP_EXTENT_LAST > > # ./f2fs_io_new fiemap 0 4 file.apk > Fiemap: offset = 0 len = 4 > logical addr. physical addr. length flags > 0 0000000000000000 000000070e08f000 0000000000004000 00001008 > > but if len equal or greater than 5, it will tag FIEMAP_EXTENT_LAST > # ./f2fs_io_new fiemap 0 5 file.apk > Fiemap: offset = 0 len = 5 > logical addr. physical addr. length flags > 0 0000000000000000 000000070e08f000 0000000000004000 00001008 > 1 0000000000004000 000000070e090000 0000000000001000 00001001 > thanks! Hi Chao, pls ignore this reply , I will confirm your questions. thanks a lot~ > > > > Thanks, > > > > > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > > --- > > > f2fs_io has been modified for testing, the length for fiemap is > > > real file size, not block number > > > --- > > > fs/f2fs/data.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > > index 90fa8ab..8c9bb42 100644 > > > --- a/fs/f2fs/data.c > > > +++ b/fs/f2fs/data.c > > > @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > > > goto out; > > > } > > > > > > - if (bytes_to_blks(inode, len) == 0) > > > - len = blks_to_bytes(inode, 1); > > > + if (len & (blks_to_bytes(inode, 1) - 1)) > > > + len = round_up(len, blks_to_bytes(inode, 1)); > > > > > > start_blk = bytes_to_blks(inode, start); > > > last_blk = bytes_to_blks(inode, start + len - 1); > >
Chao Yu <chao@kernel.org> 于2024年11月1日周五 10:36写道： > > On 2024/10/31 13:59, Zhiguo Niu wrote: > > If user give a file size as "length" parameter for fiemap > > operations, but this size is non-block size aligned, > > it will show 2 segments fiemap results even this whole file > > is contiguous on disk, such as the following results, please > > note that this f2fs_io has been modified for testing. > > > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > Fiemap: offset = 0 len = 19304 > > logical addr. physical addr. length flags > > 0 0000000000000000 0000000020baa000 0000000000004000 00001000 > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > > > > after this patch: > > ./f2fs_io fiemap 0 19304 ylog/analyzer.py > > Fiemap: offset = 0 len = 19304 > > logical addr. physical addr. length flags > > 0 0000000000000000 00000000315f3000 0000000000005000 00001000 > > Why is FIEMAP_EXTENT_LAST missing in #0 extent? As we can see it > in #1 extent before your change. Hi Chao, after I confirm, this log came from my local code and with some other modification. And after I use the f2fs original code with this patch , can see the FIEMAP_EXTENT_LAST has been tagged, the following is the fiemap log. /f2fs_io fiemap 0 19034 ylog/analyzer.py < Fiemap: offset = 0 len = 19034 logical addr. physical addr. length flags 0 0000000000000000 00000000315f3000 0000000000005000 00001001 I will update the commit msg in the next version after reviewing. Thanks a lot. > > 1 0000000000004000 0000000020bae000 0000000000001000 00001001 > > Thanks, > > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > f2fs_io has been modified for testing, the length for fiemap is > > real file size, not block number > > --- > > fs/f2fs/data.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index 90fa8ab..8c9bb42 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > > goto out; > > } > > > > - if (bytes_to_blks(inode, len) == 0) > > - len = blks_to_bytes(inode, 1); > > + if (len & (blks_to_bytes(inode, 1) - 1)) > > + len = round_up(len, blks_to_bytes(inode, 1)); > > > > start_blk = bytes_to_blks(inode, start); > > last_blk = bytes_to_blks(inode, start + len - 1); >
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 90fa8ab..8c9bb42 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1966,8 +1966,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, goto out; } - if (bytes_to_blks(inode, len) == 0) - len = blks_to_bytes(inode, 1); + if (len & (blks_to_bytes(inode, 1) - 1)) + len = round_up(len, blks_to_bytes(inode, 1)); start_blk = bytes_to_blks(inode, start); last_blk = bytes_to_blks(inode, start + len - 1);
If user give a file size as "length" parameter for fiemap operations, but this size is non-block size aligned, it will show 2 segments fiemap results even this whole file is contiguous on disk, such as the following results, please note that this f2fs_io has been modified for testing. ./f2fs_io fiemap 0 19304 ylog/analyzer.py Fiemap: offset = 0 len = 19304 logical addr. physical addr. length flags 0 0000000000000000 0000000020baa000 0000000000004000 00001000 1 0000000000004000 0000000020bae000 0000000000001000 00001001 after this patch: ./f2fs_io fiemap 0 19304 ylog/analyzer.py Fiemap: offset = 0 len = 19304 logical addr. physical addr. length flags 0 0000000000000000 00000000315f3000 0000000000005000 00001000 Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> --- f2fs_io has been modified for testing, the length for fiemap is real file size, not block number --- fs/f2fs/data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)