| 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: Wed, 2 Feb 2022 17:50:46 -0800
From: Jaegeuk Kim <jaegeuk@...nel.org>
To: Chao Yu <chao@...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 01/29, Chao Yu wrote:
> 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?
OPU is the default as F2FS was originated from LFS. Then, IPU was added to
improve performance in some cases to avoid node updates. IPU_FORCE was added
to enforce IPU all the time. So, I think you can add another IPU bit which
can relax IPU_FORCE.
>
> 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