Re: [f2fs-dev] 答复: [PATCH] f2fs: Fix data consistency in f2fs_move_file_range()

[Chao Yu <chao@kernel.org>]

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?

__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 <zhangqilong3@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