On 2024/9/23 16:54, Jan Kara wrote:
> On Wed 04-09-24 14:29:25, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>>
>> Now the beginning of the first four functions in ext4_fallocate() (punch
>> hole, zero range, insert range and collapse range) are almost the same,
>> they need to wait for the dio to finish, get filemap invalidate lock,
>> write back dirty data and finally drop page cache. Factor out a common
>> helper to do these work can reduce a lot of the redundant code.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> I like that we factor out this functionality in a common helper. But see
> below:
>
>> @@ -4731,6 +4707,52 @@ static int ext4_fallocate_check(struct inode *inode, int mode,
>> return 0;
>> }
>>
>> +int ext4_prepare_falloc(struct file *file, loff_t start, loff_t end, int mode)
>> +{
>> + struct inode *inode = file_inode(file);
>> + struct address_space *mapping = inode->i_mapping;
>> + int ret;
>> +
>> + /* Wait all existing dio workers, newcomers will block on i_rwsem */
>> + inode_dio_wait(inode);
>> + ret = file_modified(file);
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * Prevent page faults from reinstantiating pages we have released
>> + * from page cache.
>> + */
>> + filemap_invalidate_lock(mapping);
>> +
>> + ret = ext4_break_layouts(inode);
>> + if (ret)
>> + goto failed;
>> +
>> + /*
>> + * Write data that will be zeroed to preserve them when successfully
>> + * discarding page cache below but fail to convert extents.
>> + */
>> + ret = filemap_write_and_wait_range(mapping, start, end);
>
> The comment is somewhat outdated now.
Sure, will update it in next iteration.
> Also the range is wrong for collapse
> and insert range. There we need to writeout data upto the EOF because we
> truncate it below.
>
For collapse and insert range, I passed the length LLONG_MAX, which is
the same as before, this should've upto the EOF, so I think it's
right, or am I missing something?
ext4_collapse_range():
- start = round_down(offset, PAGE_SIZE);
- /* Write out all dirty pages */
- ret = filemap_write_and_wait_range(mapping, start, LLONG_MAX);
+ ret = ext4_prepare_falloc(file, round_down(offset, PAGE_SIZE),
+ LLONG_MAX, FALLOC_FL_COLLAPSE_RANGE);
ext4_insert_range():
- start = round_down(offset, PAGE_SIZE);
- /* Write out all dirty pages */
- ret = filemap_write_and_wait_range(mapping, start, LLONG_MAX);
+ ret = ext4_prepare_falloc(file, round_down(offset, PAGE_SIZE),
+ LLONG_MAX, FALLOC_FL_INSERT_RANGE);
Thanks,
Yi.
>
>> + if (ret)
>> + goto failed;
>> +
>> + /*
>> + * For insert range and collapse range, COWed private pages should
>> + * be removed since the file's logical offset will be changed, but
>> + * punch hole and zero range doesn't.
>> + */
>> + if (mode & (FALLOC_FL_INSERT_RANGE | FALLOC_FL_COLLAPSE_RANGE))
>> + truncate_pagecache(inode, start);
>> + else
>> + truncate_pagecache_range(inode, start, end);
>> +
>> + return 0;
>> +failed:
>> + filemap_invalidate_unlock(mapping);
>> + return ret;
>> +}
>
