On 2022/10/25 15:01, zhangqilong wrote:
On 2022/10/25 14:27, zhangqilong wrote:
On 2022/10/20 15:27, zhangqilong via Linux-f2fs-devel wrote:
On 2022/10/18 10:45, Zhang Qilong wrote:
In the following case:
process 1 process 2
->open A
->mmap
->read # the first time
->ioctl w/h F2FS_IOC_MOVE_RANGE
# (range A->B)
->read # the second time
How about checking B as well? Previous mapped data can still be
accessed after F2FS_IOC_MOVE_RANGE?
Hi
I have checked B as well. Previous mapped data can't be accessed
after F2FS_IOC_MOVE_RANGE.
I doubt that we didn't call flush_dcache_page() in below branch, so
user may see stall data after F2FS_IOC_MOVE_RANGE? Am I missing
something?
Hi,
You are right, it needs flush_dcache_page, but it is unnecessary here,
the __clone_blkaddrs() is called by FALLOC_FL_COLLAPSE_RANGE/
FALLOC_FL_INSERT_RANGE /F2FS_IOC_MOVE_RANGE.
->__exchange_data_block()
->__clone_blkaddrs()
f2fs_do_collapse() and f2fs_insert_range() have truncate_pagecache
after __exchange_data_block() It seem we have analyzed before. So we
only need to add a truncate operation for F2FS_IOC_MOVE_RANGE.
I mean it needs to call truncate_pagecache_range(dst, ...) in
f2fs_move_file_range() as well, right?
Yes, I think it should call truncate_pagecache_range(dst, ...) or
flush_dcache_page() here.
I submitted a patch before, it seems to be forgetten.
https://lore.kernel.org/linux-f2fs-devel/20220825024102.120651-1-zhangqilo...@huawei.com/
But, I test it w/o truncate_pagecache_range(dst, ...) or flush_dcache_page(),
user can not
see stall dst data, maybe It is a bit difficult to construct the scene for me.
Please check the condition how can we run into below else branch. I guess you
need to persist data blocks of src into a checkpoint w/ SYNC(2), then
__clone_blkaddrs() will copy data from page cache directly instead of exchanging
metadatas.
Thanks,
Thanks,
Thanks,
__clone_blkaddrs()
{
...
} else {
struct page *psrc, *pdst;
psrc = f2fs_get_lock_data_page(src_inode,
src + i, true);
if (IS_ERR(psrc))
return PTR_ERR(psrc);
pdst = f2fs_get_new_data_page(dst_inode, NULL,
dst + i,
true);
if (IS_ERR(pdst)) {
f2fs_put_page(psrc, 1);
return PTR_ERR(pdst);
}
memcpy_page(pdst, 0, psrc, 0, PAGE_SIZE);
set_page_dirty(pdst);
f2fs_put_page(pdst, 1);
f2fs_put_page(psrc, 1);
ret = f2fs_truncate_hole(src_inode,
src + i, src + i + 1);
if (ret)
return ret;
i++;
}
...
}
Thanks,
In addition, this patch could be applied to mainline if possible?
Thanks
Thanks,
We will read old data at the second time. The root cause is that
user still can see the previous source data after being moved. We
fix it by adding truncating after __exchange_data_block.
Fixes: 4dd6f977fc77 ("f2fs: support an ioctl to move a range of
data
blocks")
Signed-off-by: Zhang Qilong <zhangqilo...@huawei.com>
---
v2:
- moving truncating to the range of f2fs_lock_op()
v3:
- modify the title and commit message
---
fs/f2fs/file.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index
82cda1258227..e9dfa41baf9e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2824,6 +2824,7 @@ static int f2fs_move_file_range(struct file
*file_in,
loff_t pos_in,
goto out_src;
}
+ filemap_invalidate_lock(src->i_mapping);
f2fs_lock_op(sbi);
ret = __exchange_data_block(src, dst, pos_in >>
F2FS_BLKSIZE_BITS,
pos_out >> F2FS_BLKSIZE_BITS, @@
-2835,7
+2836,9 @@ static
int f2fs_move_file_range(struct file *file_in,
loff_t pos_in,
else if (dst_osize != dst->i_size)
f2fs_i_size_write(dst, dst_osize);
}
+ truncate_pagecache_range(src, pos_in, pos_in + len - 1);
f2fs_unlock_op(sbi);
+ filemap_invalidate_unlock(src->i_mapping);
if (src != dst)
f2fs_up_write(&F2FS_I(dst)->i_gc_rwsem[WRITE]);
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
