From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BA750C433EF for ; Fri, 4 Feb 2022 06:43:37 +0000 (UTC) Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.94.2) (envelope-from ) id 1nFsJl-0006wf-D2; Fri, 04 Feb 2022 06:43:36 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nFsJk-0006wZ-5z for linux-f2fs-devel@lists.sourceforge.net; Fri, 04 Feb 2022 06:43:35 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To: From:References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=6q0sqQMSanH1x2zMZbnMiBJHC1wWCShzzb4egDTfoxA=; b=BSce7v2cwp+4qcTgoqofItYtkZ jMlGl45MkthYX9JTBYPJfcGZ7hRxYqvT0ldCAN3MX7zOUs/cxVgtrRdAJu3O/fCL0By81cJsWbiQI ufTZzsFVTp1cHKo1qAzNqqBU0EMOOmvvo3ILpE4YU3lv/mTjpwDnsPzhe5P8Qb/Hfhe4=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=6q0sqQMSanH1x2zMZbnMiBJHC1wWCShzzb4egDTfoxA=; b=N5DEfY2BGEJ/lXvzxnTT2pZ+9I TRPd/ezGa1z2/M3YfMmQeQTf6MnExFYSbtNteJId5DWyO3W6T5dJ4Si45SIib7aq29DT16PBybiM2 B38tdMiGulh477vYt/l9AkXymmEibwhTd08G1sOKvHZEuLubSWHVwgDxGMa0JFfYNrc0=; Received: from dfw.source.kernel.org ([139.178.84.217]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.94.2) id 1nFsJh-00GWPi-V4 for linux-f2fs-devel@lists.sourceforge.net; Fri, 04 Feb 2022 06:43:34 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 8696861BD1 for ; Fri, 4 Feb 2022 06:43:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B707C004E1; Fri, 4 Feb 2022 06:43:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1643957008; bh=/VdwXlVq7dzO5Ru/fFP2SsA88ZZFmo+3IMjz2nvDvI4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=tfGAbdc/M1Q78EMOF2bCoeuxSXr50jNlIuVjvkqNWA9gJBlsCC0SXjOXhQtr755kt dHMoDoJnPZUS4Iu6or5isBxmZcvghXvqs94Y27Nw2VDbFvkLsV3gEPjT6tPIYqRes0 UcdRRFQJHF9OdWf9FMpD35fJ/op8aiwi/NLeGV9EbEog5bHZtDqhbFQ6sJCDFfyLNB z/GPTKL403O5u9z/HK23GT3QCnTEx8MltTScA+quzWLZM6orK8dbRwzK/54HQUa387 h6bL5Kqcm4CUbsCjK2typwo7wy7//O/m32Eb5fVCIJK1yAKgudSDfMUs+uy9hi1WkG XN7HikkReaU2Q== Message-ID: <260bf309-9712-9696-49d5-3fe156bc319c@kernel.org> Date: Fri, 4 Feb 2022 14:43:24 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Content-Language: en-US To: Jaegeuk Kim References: <20220204010539.42672-1-chao@kernel.org> From: Chao Yu In-Reply-To: X-Headers-End: 1nFsJh-00GWPi-V4 Subject: Re: [f2fs-dev] [PATCH v2] f2fs: introduce F2FS_IPU_DEFRAG to allow file defragment in IPU mode X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On 2022/2/4 14:12, Jaegeuk Kim wrote: > On 02/04, 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 >> >> Then we may fail to defragment file via ioctl due to IPU policy check, >> it doesn't make sense, let's introduce a new IPU policy to allow OPU >> during file defragmentation, by default use IPU_DEFRAG instead of >> IPU_FORCE in small-sized volume for android device. >> >> Signed-off-by: Chao Yu >> --- >> v2: >> - handle F2FS_IPU_DEFRAG before F2FS_IPU_FORCE in check_inplace_update_policy(). >> Documentation/ABI/testing/sysfs-fs-f2fs | 3 ++- >> fs/f2fs/data.c | 18 +++++++++++++----- >> fs/f2fs/f2fs.h | 3 ++- >> fs/f2fs/file.c | 18 +++++++++++------- >> fs/f2fs/segment.h | 8 ++++---- >> fs/f2fs/super.c | 2 +- >> 6 files changed, 33 insertions(+), 19 deletions(-) >> >> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs >> index ce8103f522cb..d8abb674ee86 100644 >> --- a/Documentation/ABI/testing/sysfs-fs-f2fs >> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs >> @@ -55,8 +55,9 @@ Description: Controls the in-place-update policy. >> 0x04 F2FS_IPU_UTIL >> 0x08 F2FS_IPU_SSR_UTIL >> 0x10 F2FS_IPU_FSYNC >> - 0x20 F2FS_IPU_ASYNC, >> + 0x20 F2FS_IPU_ASYNC >> 0x40 F2FS_IPU_NOCACHE >> + 0x80 F2FS_IPU_DEFRAG > > How about F2FS_IPU_HONOR_OPU_WRITE? Looks fine. > >> ==== ================= >> >> Refer segment.h for details. >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index b3c152de4bba..3c0f001e3fe1 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -2460,6 +2460,9 @@ static inline bool check_inplace_update_policy(struct inode *inode, >> struct f2fs_sb_info *sbi = F2FS_I_SB(inode); >> unsigned int policy = SM_I(sbi)->ipu_policy; >> >> + if (policy & (0x1 << F2FS_IPU_DEFRAG) && >> + is_inode_flag_set(inode, FI_DEFRAG_PROCESS)) >> + return false; >> if (policy & (0x1 << F2FS_IPU_FORCE)) >> return true; >> if (policy & (0x1 << F2FS_IPU_SSR) && f2fs_need_SSR(sbi)) >> @@ -2530,6 +2533,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 +3160,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); >> @@ -3729,6 +3735,7 @@ static int f2fs_migrate_blocks(struct inode *inode, block_t start_blk, >> filemap_invalidate_lock(inode->i_mapping); >> >> set_inode_flag(inode, FI_ALIGNED_WRITE); >> + set_inode_flag(inode, FI_DEFRAG_PROCESS); >> >> for (; secidx < end_sec; secidx++) { >> f2fs_down_write(&sbi->pin_sem); >> @@ -3737,7 +3744,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; >> @@ -3754,7 +3761,7 @@ 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); >> >> ret = filemap_fdatawrite(inode->i_mapping); >> >> @@ -3765,7 +3772,8 @@ 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_DEFRAG_PROCESS); >> 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 6ddb98ff0b7c..5af415208488 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -737,7 +737,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 */ > > This is used for f2fs_migrate_blocks for swap file. Can we rename to > FI_SKIP_WRITES that makes skipping writeback pages? Sure. > >> + FI_DEFRAG_PROCESS, /* indicate defragment is processing */ > > How about FI_OPU_WRITE? Looks fine. > >> 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..3a573125002b 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,13 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, >> >> inode_lock(inode); >> >> + /* if in-place-update policy is enabled, don't waste time here */ >> + set_inode_flag(inode, FI_DEFRAG_PROCESS); >> + if (f2fs_should_update_inplace(inode, NULL)) { >> + 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 +2654,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,15 +2679,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); >> >> err = filemap_fdatawrite(inode->i_mapping); >> if (err) >> goto out; >> } >> clear_out: >> - clear_inode_flag(inode, FI_DO_DEFRAG); >> + clear_inode_flag(inode, FI_DEFRAG_PREPARE); >> out: >> + clear_inode_flag(inode, FI_DEFRAG_PROCESS); >> inode_unlock(inode); >> if (!err) >> range->len = (u64)total << PAGE_SHIFT; >> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h >> index 0291cd55cf09..a9373861ab1c 100644 >> --- a/fs/f2fs/segment.h >> +++ b/fs/f2fs/segment.h >> @@ -651,7 +651,8 @@ static inline int utilization(struct f2fs_sb_info *sbi) >> * pages over min_fsync_blocks. (=default option) >> * F2FS_IPU_ASYNC - do IPU given by asynchronous write requests. >> * F2FS_IPU_NOCACHE - disable IPU bio cache. >> - * F2FS_IPUT_DISABLE - disable IPU. (=default option in LFS mode) >> + * F2FS_IPU_DEFRAG - allow OPU during file defragmentation. >> + * F2FS_IPU_DISABLE - disable IPU. (=default option in LFS mode) >> */ >> #define DEF_MIN_IPU_UTIL 70 >> #define DEF_MIN_FSYNC_BLOCKS 8 >> @@ -667,6 +668,7 @@ enum { >> F2FS_IPU_FSYNC, >> F2FS_IPU_ASYNC, >> F2FS_IPU_NOCACHE, >> + F2FS_IPU_DEFRAG, >> }; >> >> static inline unsigned int curseg_segno(struct f2fs_sb_info *sbi, >> @@ -675,9 +677,7 @@ static inline unsigned int curseg_segno(struct f2fs_sb_info *sbi, >> struct curseg_info *curseg = CURSEG_I(sbi, type); >> return curseg->segno; >> } >> - >> -static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, >> - int type) >> +static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, int type) > > Don't do this. Oops, let me remove this unrelated change. > >> { >> struct curseg_info *curseg = CURSEG_I(sbi, type); >> return curseg->alloc_type; >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index 9af6c20532ec..cc0429279b4c 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -3957,7 +3957,7 @@ static void f2fs_tuning_parameters(struct f2fs_sb_info *sbi) >> F2FS_OPTION(sbi).alloc_mode = ALLOC_MODE_REUSE; >> if (f2fs_block_unit_discard(sbi)) >> sm_i->dcc_info->discard_granularity = 1; >> - sm_i->ipu_policy = 1 << F2FS_IPU_FORCE; > > No, I think we should have both. Alright. Thanks, > >> + sm_i->ipu_policy = 1 << F2FS_IPU_DEFRAG; >> } >> >> sbi->readdir_ra = 1; >> -- >> 2.32.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel