| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 29 Jan 2022 09:39:45 +0800
From: Chao Yu <chao@...nel.org>
To: Jaegeuk Kim <jaegeuk@...nel.org>
Cc: linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: don't check IPU policy during file defragment
On 2022/1/29 8:35, Jaegeuk Kim wrote:
> On 01/28, Chao Yu wrote:
>> Once IPU policy is enabled in some cases:
>> a) f2fs forces to use F2FS_IPU_FORCE in a small-sized volume
>> b) user configures IPU policy via sysfs
>
> It's contradictory to each other. How about just letting users decide to do what
> they want to do? IOWs, if user wants to do defrag, they should change ipu_policy
> on the fly.
How about the case user wants to do defrag for specified files, and also use
ipu_policy for other common writes?
IMO, it may be not completely contradictory, if we try to treat userspace defraging
requirement as specified OPU requirement, like kernel-side OPU requirement from
node write or GC write.
Or maybe we can introduce a new ipu_policy which allow triggering OPU when defraging
triggered from userspace?
Thanks,
>
>>
>> Then we may fail to defragment file via ioctl due to IPU policy check,
>> it doesn't make sense, so let's change to use IPU policy for common
>> data writeback, rather than for specific data writeback, e.g. GC,
>> defragment, and so on.
>>
>> Signed-off-by: Chao Yu <chao@...nel.org>
>> ---
>> fs/f2fs/data.c | 15 ++++++++++-----
>> fs/f2fs/f2fs.h | 3 ++-
>> fs/f2fs/file.c | 17 ++++++++++-------
>> 3 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 0f124e8de1d4..bce7ecac8976 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2530,6 +2530,9 @@ bool f2fs_should_update_outplace(struct inode *inode, struct f2fs_io_info *fio)
>> if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>> return true;
>>
>> + if (is_inode_flag_set(inode, FI_DEFRAG_PROCESS))
>> + return true;
>> +
>> if (fio) {
>> if (page_private_gcing(fio->page))
>> return true;
>> @@ -3154,8 +3157,8 @@ static int __f2fs_write_data_pages(struct address_space *mapping,
>> f2fs_available_free_memory(sbi, DIRTY_DENTS))
>> goto skip_write;
>>
>> - /* skip writing during file defragment */
>> - if (is_inode_flag_set(inode, FI_DO_DEFRAG))
>> + /* skip writing in file defragment preparing stage */
>> + if (is_inode_flag_set(inode, FI_DEFRAG_PREPARE))
>> goto skip_write;
>>
>> trace_f2fs_writepages(mapping->host, wbc, DATA);
>> @@ -3733,7 +3736,7 @@ static int f2fs_migrate_blocks(struct inode *inode, block_t start_blk,
>> f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_PINNED, false);
>> f2fs_unlock_op(sbi);
>>
>> - set_inode_flag(inode, FI_DO_DEFRAG);
>> + set_inode_flag(inode, FI_DEFRAG_PREPARE);
>>
>> for (blkofs = 0; blkofs < blk_per_sec; blkofs++) {
>> struct page *page;
>> @@ -3750,9 +3753,11 @@ static int f2fs_migrate_blocks(struct inode *inode, block_t start_blk,
>> f2fs_put_page(page, 1);
>> }
>>
>> - clear_inode_flag(inode, FI_DO_DEFRAG);
>> + clear_inode_flag(inode, FI_DEFRAG_PREPARE);
>>
>> + set_inode_flag(inode, FI_DEFRAG_PROCESS);
>> ret = filemap_fdatawrite(inode->i_mapping);
>> + clear_inode_flag(inode, FI_DEFRAG_PROCESS);
>>
>> f2fs_up_write(&sbi->pin_sem);
>>
>> @@ -3761,7 +3766,7 @@ static int f2fs_migrate_blocks(struct inode *inode, block_t start_blk,
>> }
>>
>> done:
>> - clear_inode_flag(inode, FI_DO_DEFRAG);
>> + clear_inode_flag(inode, FI_DEFRAG_PREPARE);
>> clear_inode_flag(inode, FI_ALIGNED_WRITE);
>>
>> filemap_invalidate_unlock(inode->i_mapping);
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 8178a9152e49..4b905059a81e 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -733,7 +733,8 @@ enum {
>> FI_DROP_CACHE, /* drop dirty page cache */
>> FI_DATA_EXIST, /* indicate data exists */
>> FI_INLINE_DOTS, /* indicate inline dot dentries */
>> - FI_DO_DEFRAG, /* indicate defragment is running */
>> + FI_DEFRAG_PREPARE, /* indicate defragment is preparing */
>> + FI_DEFRAG_PROCESS, /* indicate defragment is processing */
>> FI_DIRTY_FILE, /* indicate regular/symlink has dirty pages */
>> FI_PREALLOCATED_ALL, /* all blocks for write were preallocated */
>> FI_HOT_DATA, /* indicate file is hot */
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 6ccdd6e347e2..696f4a175228 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2559,10 +2559,6 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>> bool fragmented = false;
>> int err;
>>
>> - /* if in-place-update policy is enabled, don't waste time here */
>> - if (f2fs_should_update_inplace(inode, NULL))
>> - return -EINVAL;
>> -
>> pg_start = range->start >> PAGE_SHIFT;
>> pg_end = (range->start + range->len) >> PAGE_SHIFT;
>>
>> @@ -2570,6 +2566,11 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>>
>> inode_lock(inode);
>>
>> + if (f2fs_is_pinned_file(inode)) {
>> + err = -EINVAL;
>> + goto out;
>> + }
>> +
>> /* writeback all dirty pages in the range */
>> err = filemap_write_and_wait_range(inode->i_mapping, range->start,
>> range->start + range->len - 1);
>> @@ -2651,7 +2652,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>> goto check;
>> }
>>
>> - set_inode_flag(inode, FI_DO_DEFRAG);
>> + set_inode_flag(inode, FI_DEFRAG_PREPARE);
>>
>> idx = map.m_lblk;
>> while (idx < map.m_lblk + map.m_len && cnt < blk_per_seg) {
>> @@ -2676,14 +2677,16 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>> if (map.m_lblk < pg_end && cnt < blk_per_seg)
>> goto do_map;
>>
>> - clear_inode_flag(inode, FI_DO_DEFRAG);
>> + clear_inode_flag(inode, FI_DEFRAG_PREPARE);
>>
>> + set_inode_flag(inode, FI_DEFRAG_PROCESS);
>> err = filemap_fdatawrite(inode->i_mapping);
>> + clear_inode_flag(inode, FI_DEFRAG_PROCESS);
>> if (err)
>> goto out;
>> }
>> clear_out:
>> - clear_inode_flag(inode, FI_DO_DEFRAG);
>> + clear_inode_flag(inode, FI_DEFRAG_PREPARE);
>> out:
>> inode_unlock(inode);
>> if (!err)
>> --
>> 2.25.1
Powered by blists - more mailing lists