> 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.
> __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
