Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
On Mon, Dec 12, 2022 at 05:22:41PM -0800, Jaegeuk Kim wrote:
> > struct dnode_of_data dn;
> > @@ -1484,11 +1483,7 @@ int f2fs_map_blocks(struct inode *inode, struct
> > f2fs_map_blocks *map,
> > pgofs = (pgoff_t)map->m_lblk;
> > end = pgofs + maxblocks;
> >
> > - if (!create && f2fs_lookup_extent_cache(inode, pgofs, )) {
> > - if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
>
> Any reason to remove this condition?
>
> Thanks,
>
> > - map->m_may_create)
With "this condition" I guess you mean the:
if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
map->m_may_create)
goto next_dnode;
?
Now that the !create check above is replaced with !map->m_may_create
above, it is dead code, as the map->m_may_create part of the conditions
must be false.
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [GIT PULL] fscrypt updates for 6.2
The pull request you sent on Sun, 11 Dec 2022 20:45:54 -0800: > https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git tags/fscrypt-for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8129bac60f30936d2339535841db5b66d0520a67 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [GIT PULL] fsverity updates for 6.2
The pull request you sent on Sun, 11 Dec 2022 20:48:35 -0800: > https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git tags/fsverity-for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/ad0d9da164cb52e62637e427517b2060dc956a2d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: don't set FI_COMPRESS_RELEASED if file is not compressed
However, 'f2fs_compressed_file()' only determines whether the file can be compressed, not whether the file has been compressed. As far as I know, when compress_mode is user, files marked FI_COMPRESSED_FILE will be compressed only after 'f2fs_ioc_compress_file()' is called. On Mon, Dec 12, 2022 at 03:05:08PM -0800, Jaegeuk Kim wrote: > On 12/12, zhoudan wrote: > > Maybe I'm not describing it clearly enough, but I think there is > > something wrong with the logic here.The 'f2fs_release_compress_blocks' > > method does not determine if the file is compressed, but simply adds > > the FI_COMPRESS_RELEASED flag. > > I firstly lost your point since f2fs_release_compress_blocks() checked > f2fs_compressed_file(). > > > In particular, in the current Android system, when the application is > > installed, the release interface is called by default to release the > > storage marked as compressed, without checking whether the file is > > actually compressed. In this case, when compress_mode is set to user, > > calling the compress interface returns ENVAL and the file cannot be > > compressed. > > So I think the implementation of release needs to be modified, and > > only set FI_COMPRESS_RELEASED when it's really compressed and the > > storage is released. > > > > On Fri, Dec 09, 2022 at 12:19:44PM -0800, Jaegeuk Kim wrote: > > > On 12/08, zhoudan8 wrote: > > > > In compress_mode=user, f2fs_release_compress_blocks() > > > > does not verify whether it has been compressed and > > > > sets FI_COMPRESS_RELEASED directly. which will lead to > > > > return -EINVAL after calling compress. > > > > To fix it,let's do not set FI_COMPRESS_RELEASED if file > > > > is not compressed. > > > > > > Do you mean you want to avoid EINVAL on a file having FI_COMPRESS_RELEASED > > > with zero i_compr_blokcs? > > > > > > I think the current logic is giving the error on a released file already. > > > > > > > > > > > Signed-off-by: zhoudan8 > > > > --- > > > > fs/f2fs/file.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > > > > index 82cda1258227..f32910077df6 100644 > > > > --- a/fs/f2fs/file.c > > > > +++ b/fs/f2fs/file.c > > > > @@ -3451,14 +3451,13 @@ static int f2fs_release_compress_blocks(struct > > > > file *filp, unsigned long arg) > > > > ret = filemap_write_and_wait_range(inode->i_mapping, 0, > > > > LLONG_MAX); > > > > if (ret) > > > > goto out; > > > > - > > > > - set_inode_flag(inode, FI_COMPRESS_RELEASED); > > > > inode->i_ctime = current_time(inode); > > > > f2fs_mark_inode_dirty_sync(inode, true); > > > > > > > > if (!atomic_read(_I(inode)->i_compr_blocks)) > > > > goto out; > > > > > > > > + set_inode_flag(inode, FI_COMPRESS_RELEASED); > > > > f2fs_down_write(_I(inode)->i_gc_rwsem[WRITE]); > > > > filemap_invalidate_lock(inode->i_mapping); > > > > > > > > -- > > > > 2.38.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: do some cleanup for f2fs module init
On 2022/12/13 9:37, Jaegeuk Kim wrote:
On 12/13, Chao Yu wrote:
On 2022/12/13 6:53, Jaegeuk Kim wrote:
On 12/11, Chao Yu wrote:
On 2022/11/25 19:47, Yangtao Li wrote:
Just for cleanup, no functional changes.
Signed-off-by: Yangtao Li
---
fs/f2fs/compress.c | 46 ++
fs/f2fs/data.c | 14 --
fs/f2fs/gc.c | 4 +---
fs/f2fs/recovery.c | 4 +---
fs/f2fs/super.c| 8 ++--
5 files changed, 14 insertions(+), 62 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d315c2de136f..f920ba8e0e85 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -567,10 +567,7 @@ MODULE_PARM_DESC(num_compress_pages,
int f2fs_init_compress_mempool(void)
{
compress_page_pool = mempool_create_page_pool(num_compress_pages, 0);
- if (!compress_page_pool)
- return -ENOMEM;
-
- return 0;
+ return compress_page_pool ? 0 : -ENOMEM;
I don't think this needs cleanup, other part looks good to me.
What is the point here comparing to the below? fyi; I picked this change.
IIUC, the question is for Yangtao? :P
Heh, to you. :) I think either looks fine. Hence,
Ah... alright.
I comment on this cleanup, due to I don't see any benefit, since both
implementations
are very common in kernel codes.
I'm fine with this patch.
Okay.
Thanks,
Thanks,
Thanks,
}
void f2fs_destroy_compress_mempool(void)
@@ -1983,9 +1980,7 @@ int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi)
sbi->page_array_slab = f2fs_kmem_cache_create(slab_name,
sbi->page_array_slab_size);
- if (!sbi->page_array_slab)
- return -ENOMEM;
- return 0;
+ return sbi->page_array_slab ? 0 : -ENOMEM;
}
void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi)
@@ -1993,53 +1988,24 @@ void f2fs_destroy_page_array_cache(struct f2fs_sb_info
*sbi)
kmem_cache_destroy(sbi->page_array_slab);
}
-static int __init f2fs_init_cic_cache(void)
+int __init f2fs_init_compress_cache(void)
{
cic_entry_slab = f2fs_kmem_cache_create("f2fs_cic_entry",
sizeof(struct compress_io_ctx));
if (!cic_entry_slab)
return -ENOMEM;
- return 0;
-}
-
-static void f2fs_destroy_cic_cache(void)
-{
- kmem_cache_destroy(cic_entry_slab);
-}
-
-static int __init f2fs_init_dic_cache(void)
-{
dic_entry_slab = f2fs_kmem_cache_create("f2fs_dic_entry",
sizeof(struct decompress_io_ctx));
if (!dic_entry_slab)
- return -ENOMEM;
- return 0;
-}
-
-static void f2fs_destroy_dic_cache(void)
-{
- kmem_cache_destroy(dic_entry_slab);
-}
-
-int __init f2fs_init_compress_cache(void)
-{
- int err;
-
- err = f2fs_init_cic_cache();
- if (err)
- goto out;
- err = f2fs_init_dic_cache();
- if (err)
goto free_cic;
return 0;
free_cic:
- f2fs_destroy_cic_cache();
-out:
+ kmem_cache_destroy(cic_entry_slab);
return -ENOMEM;
}
void f2fs_destroy_compress_cache(void)
{
- f2fs_destroy_dic_cache();
- f2fs_destroy_cic_cache();
+ kmem_cache_destroy(dic_entry_slab);
+ kmem_cache_destroy(cic_entry_slab);
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 560fa80590e9..35c19248b1e2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -39,10 +39,8 @@ static struct bio_set f2fs_bioset;
int __init f2fs_init_bioset(void)
{
- if (bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
- 0, BIOSET_NEED_BVECS))
- return -ENOMEM;
- return 0;
+ return bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
+ 0, BIOSET_NEED_BVECS);
}
void f2fs_destroy_bioset(void)
@@ -4090,9 +4088,7 @@ int f2fs_init_post_read_wq(struct f2fs_sb_info *sbi)
sbi->post_read_wq = alloc_workqueue("f2fs_post_read_wq",
WQ_UNBOUND | WQ_HIGHPRI,
num_online_cpus());
- if (!sbi->post_read_wq)
- return -ENOMEM;
- return 0;
+ return sbi->post_read_wq ? 0 : -ENOMEM;
}
void f2fs_destroy_post_read_wq(struct f2fs_sb_info *sbi)
@@ -4105,9 +4101,7 @@ int __init f2fs_init_bio_entry_cache(void)
{
bio_entry_slab = f2fs_kmem_cache_create("f2fs_bio_entry_slab",
sizeof(struct bio_entry));
- if (!bio_entry_slab)
- return -ENOMEM;
- return 0;
+ return bio_entry_slab ? 0 : -ENOMEM;
}
void f2fs_destroy_bio_entry_cache(void)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 0f967b1e98f2..4b0d2fa3a769 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1903,9 +1903,7 @@ int __init f2fs_create_garbage_collection_cache(void)
Re: [f2fs-dev] [PATCH] f2fs: fix iostat parameter for discard
On 2022/12/13 6:59, Jaegeuk Kim wrote:
On 12/11, Chao Yu wrote:
On 2022/12/5 22:56, Yangtao Li wrote:
Just like other data we count uses the number of bytes as the basic unit,
but discard uses the number of cmds as the statistical unit. In fact the
discard command contains the number of blocks, so let's change to the
number of bytes as the base unit.
Fixes: b0af6d491a6b ("f2fs: add app/fs io stat")
Signed-off-by: Yangtao Li
---
fs/f2fs/segment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9486ca49ecb1..bc262e17b279 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1181,7 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
atomic_inc(>issued_discard);
- f2fs_update_iostat(sbi, NULL, FS_DISCARD, 1);
+ f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
In order to avoid breaking its usage of application, how about keeping
FS_DISCARD as it is, and add FS_DISCARD_IO to account discard bytes?
I picked this to match the f2fs_update_iostat() definition. Do we know
how many applications will be affected? I don't have any at a glance in
Android tho.
I guess there is some private scripts in OEM rely on this, and I don't
see any non-Android users using this.
void f2fs_update_iostat(struct f2fs_sb_info *sbi, struct inode *inode,
enum iostat_type type, unsigned long long io_bytes)
What do you think of extending this function to support io_counts?
void f2fs_update_iostat(struct f2fs_sb_info *sbi, struct inode *inode,
enum iostat_type type, unsigned long long io_bytes,
unsigned long long io_counts)
Thanks,
Thanks,
lstart += len;
start += len;
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
On 12/13, Chao Yu wrote:
> On 2022/12/13 6:45, Jaegeuk Kim wrote:
> > On 12/12, Chao Yu wrote:
> > > On 2022/12/12 22:14, Yangtao Li wrote:
> > > > Hi Chao,
> > > >
> > > > > The difference here is, if we use f2fs_realtime_discard_enable() in
> > > > > f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag
> > > > > when discard option is enable and device supports discard.
> > > >
> > > > > But actually, if discard option is disabled, we still needs to give
> > > > > put_super() a chance to write checkpoint w/ CP_TRIMMED flag.
> > > >
> > > > Why do we still have to set the CP_TRIMMED flag when the discard opt is
> > > > not set.
> > > > Did I miss something?
> > >
> > > Hi Yangtao,
> > >
> > > I guess it's up to scenario. e.g.
> > >
> > > mount w/ nodiscard and use FITRIM to trigger in-batch discard,
> > > if we set CP_TRIMMED flag during umount, next time, after mount
> > > w/ discard, it doesn't to issue redundant discard.
> >
> > If fitrim was called with a range, we can get a wrong FI_TRIMMED flag.
> > Isn't it
>
> We can set CP_TRIMMED flag only if fitrim was called on full range w/ 4k
> granularity,
> due to it will check sbi->discard_blks variable to make sure there is no
> range we
> haven't trimmed.
>
> > better to get a full discard range after remount even though some are
> > redundant?
>
> If nodiscard is set, and sbi->discard_blks becomes zero, it says a full range
> fitrim
> was been triggered.
That gives another assumption, and I prefer to make it simple.
>
> So, previous check condition has no problem, right?
>
> if ((f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) &&
> !sbi->discard_blks && !dropped) {
>
> Thanks,
>
> >
> > >
> > > Thanks,
> > >
> > > >
> > > > Thx,
> > > > Yangtao
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: do some cleanup for f2fs module init
On 12/13, Chao Yu wrote:
> On 2022/12/13 6:53, Jaegeuk Kim wrote:
> > On 12/11, Chao Yu wrote:
> > > On 2022/11/25 19:47, Yangtao Li wrote:
> > > > Just for cleanup, no functional changes.
> > > >
> > > > Signed-off-by: Yangtao Li
> > > > ---
> > > >fs/f2fs/compress.c | 46
> > > > ++
> > > >fs/f2fs/data.c | 14 --
> > > >fs/f2fs/gc.c | 4 +---
> > > >fs/f2fs/recovery.c | 4 +---
> > > >fs/f2fs/super.c| 8 ++--
> > > >5 files changed, 14 insertions(+), 62 deletions(-)
> > > >
> > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > index d315c2de136f..f920ba8e0e85 100644
> > > > --- a/fs/f2fs/compress.c
> > > > +++ b/fs/f2fs/compress.c
> > > > @@ -567,10 +567,7 @@ MODULE_PARM_DESC(num_compress_pages,
> > > >int f2fs_init_compress_mempool(void)
> > > >{
> > > > compress_page_pool =
> > > > mempool_create_page_pool(num_compress_pages, 0);
> > > > - if (!compress_page_pool)
> > > > - return -ENOMEM;
> > > > -
> > > > - return 0;
> > > > + return compress_page_pool ? 0 : -ENOMEM;
> > >
> > > I don't think this needs cleanup, other part looks good to me.
> >
> > What is the point here comparing to the below? fyi; I picked this change.
>
> IIUC, the question is for Yangtao? :P
Heh, to you. :) I think either looks fine. Hence, I'm fine with this patch.
>
> Thanks,
>
> >
> > >
> > > Thanks,
> > >
> > > >}
> > > >void f2fs_destroy_compress_mempool(void)
> > > > @@ -1983,9 +1980,7 @@ int f2fs_init_page_array_cache(struct
> > > > f2fs_sb_info *sbi)
> > > > sbi->page_array_slab = f2fs_kmem_cache_create(slab_name,
> > > > sbi->page_array_slab_size);
> > > > - if (!sbi->page_array_slab)
> > > > - return -ENOMEM;
> > > > - return 0;
> > > > + return sbi->page_array_slab ? 0 : -ENOMEM;
> > > >}
> > > >void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi)
> > > > @@ -1993,53 +1988,24 @@ void f2fs_destroy_page_array_cache(struct
> > > > f2fs_sb_info *sbi)
> > > > kmem_cache_destroy(sbi->page_array_slab);
> > > >}
> > > > -static int __init f2fs_init_cic_cache(void)
> > > > +int __init f2fs_init_compress_cache(void)
> > > >{
> > > > cic_entry_slab = f2fs_kmem_cache_create("f2fs_cic_entry",
> > > > sizeof(struct compress_io_ctx));
> > > > if (!cic_entry_slab)
> > > > return -ENOMEM;
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static void f2fs_destroy_cic_cache(void)
> > > > -{
> > > > - kmem_cache_destroy(cic_entry_slab);
> > > > -}
> > > > -
> > > > -static int __init f2fs_init_dic_cache(void)
> > > > -{
> > > > dic_entry_slab = f2fs_kmem_cache_create("f2fs_dic_entry",
> > > > sizeof(struct
> > > > decompress_io_ctx));
> > > > if (!dic_entry_slab)
> > > > - return -ENOMEM;
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static void f2fs_destroy_dic_cache(void)
> > > > -{
> > > > - kmem_cache_destroy(dic_entry_slab);
> > > > -}
> > > > -
> > > > -int __init f2fs_init_compress_cache(void)
> > > > -{
> > > > - int err;
> > > > -
> > > > - err = f2fs_init_cic_cache();
> > > > - if (err)
> > > > - goto out;
> > > > - err = f2fs_init_dic_cache();
> > > > - if (err)
> > > > goto free_cic;
> > > > return 0;
> > > >free_cic:
> > > > - f2fs_destroy_cic_cache();
> > > > -out:
> > > > + kmem_cache_destroy(cic_entry_slab);
> > > > return -ENOMEM;
> > > >}
> > > >void f2fs_destroy_compress_cache(void)
> > > >{
> > > > - f2fs_destroy_dic_cache();
> > > > - f2fs_destroy_cic_cache();
> > > > + kmem_cache_destroy(dic_entry_slab);
> > > > + kmem_cache_destroy(cic_entry_slab);
> > > >}
> > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > index 560fa80590e9..35c19248b1e2 100644
> > > > --- a/fs/f2fs/data.c
> > > > +++ b/fs/f2fs/data.c
> > > > @@ -39,10 +39,8 @@ static struct bio_set f2fs_bioset;
> > > >int __init f2fs_init_bioset(void)
> > > >{
> > > > - if (bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
> > > > - 0, BIOSET_NEED_BVECS))
> > > > - return -ENOMEM;
> > > > - return 0;
> > > > + return bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
> > > > + 0, BIOSET_NEED_BVECS);
> > > >}
> > > >void f2fs_destroy_bioset(void)
> > > > @@ -4090,9 +4088,7 @@ int f2fs_init_post_read_wq(struct f2fs_sb_info
> > > > *sbi)
> > > > sbi->post_read_wq = alloc_workqueue("f2fs_post_read_wq",
> > > > WQ_UNBOUND |
> > > > WQ_HIGHPRI,
> > > >
Re: [f2fs-dev] [PATCH] f2fs: do some cleanup for f2fs module init
On 2022/12/13 6:53, Jaegeuk Kim wrote:
On 12/11, Chao Yu wrote:
On 2022/11/25 19:47, Yangtao Li wrote:
Just for cleanup, no functional changes.
Signed-off-by: Yangtao Li
---
fs/f2fs/compress.c | 46 ++
fs/f2fs/data.c | 14 --
fs/f2fs/gc.c | 4 +---
fs/f2fs/recovery.c | 4 +---
fs/f2fs/super.c| 8 ++--
5 files changed, 14 insertions(+), 62 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d315c2de136f..f920ba8e0e85 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -567,10 +567,7 @@ MODULE_PARM_DESC(num_compress_pages,
int f2fs_init_compress_mempool(void)
{
compress_page_pool = mempool_create_page_pool(num_compress_pages, 0);
- if (!compress_page_pool)
- return -ENOMEM;
-
- return 0;
+ return compress_page_pool ? 0 : -ENOMEM;
I don't think this needs cleanup, other part looks good to me.
What is the point here comparing to the below? fyi; I picked this change.
IIUC, the question is for Yangtao? :P
Thanks,
Thanks,
}
void f2fs_destroy_compress_mempool(void)
@@ -1983,9 +1980,7 @@ int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi)
sbi->page_array_slab = f2fs_kmem_cache_create(slab_name,
sbi->page_array_slab_size);
- if (!sbi->page_array_slab)
- return -ENOMEM;
- return 0;
+ return sbi->page_array_slab ? 0 : -ENOMEM;
}
void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi)
@@ -1993,53 +1988,24 @@ void f2fs_destroy_page_array_cache(struct f2fs_sb_info
*sbi)
kmem_cache_destroy(sbi->page_array_slab);
}
-static int __init f2fs_init_cic_cache(void)
+int __init f2fs_init_compress_cache(void)
{
cic_entry_slab = f2fs_kmem_cache_create("f2fs_cic_entry",
sizeof(struct compress_io_ctx));
if (!cic_entry_slab)
return -ENOMEM;
- return 0;
-}
-
-static void f2fs_destroy_cic_cache(void)
-{
- kmem_cache_destroy(cic_entry_slab);
-}
-
-static int __init f2fs_init_dic_cache(void)
-{
dic_entry_slab = f2fs_kmem_cache_create("f2fs_dic_entry",
sizeof(struct decompress_io_ctx));
if (!dic_entry_slab)
- return -ENOMEM;
- return 0;
-}
-
-static void f2fs_destroy_dic_cache(void)
-{
- kmem_cache_destroy(dic_entry_slab);
-}
-
-int __init f2fs_init_compress_cache(void)
-{
- int err;
-
- err = f2fs_init_cic_cache();
- if (err)
- goto out;
- err = f2fs_init_dic_cache();
- if (err)
goto free_cic;
return 0;
free_cic:
- f2fs_destroy_cic_cache();
-out:
+ kmem_cache_destroy(cic_entry_slab);
return -ENOMEM;
}
void f2fs_destroy_compress_cache(void)
{
- f2fs_destroy_dic_cache();
- f2fs_destroy_cic_cache();
+ kmem_cache_destroy(dic_entry_slab);
+ kmem_cache_destroy(cic_entry_slab);
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 560fa80590e9..35c19248b1e2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -39,10 +39,8 @@ static struct bio_set f2fs_bioset;
int __init f2fs_init_bioset(void)
{
- if (bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
- 0, BIOSET_NEED_BVECS))
- return -ENOMEM;
- return 0;
+ return bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
+ 0, BIOSET_NEED_BVECS);
}
void f2fs_destroy_bioset(void)
@@ -4090,9 +4088,7 @@ int f2fs_init_post_read_wq(struct f2fs_sb_info *sbi)
sbi->post_read_wq = alloc_workqueue("f2fs_post_read_wq",
WQ_UNBOUND | WQ_HIGHPRI,
num_online_cpus());
- if (!sbi->post_read_wq)
- return -ENOMEM;
- return 0;
+ return sbi->post_read_wq ? 0 : -ENOMEM;
}
void f2fs_destroy_post_read_wq(struct f2fs_sb_info *sbi)
@@ -4105,9 +4101,7 @@ int __init f2fs_init_bio_entry_cache(void)
{
bio_entry_slab = f2fs_kmem_cache_create("f2fs_bio_entry_slab",
sizeof(struct bio_entry));
- if (!bio_entry_slab)
- return -ENOMEM;
- return 0;
+ return bio_entry_slab ? 0 : -ENOMEM;
}
void f2fs_destroy_bio_entry_cache(void)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 0f967b1e98f2..4b0d2fa3a769 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1903,9 +1903,7 @@ int __init f2fs_create_garbage_collection_cache(void)
{
victim_entry_slab = f2fs_kmem_cache_create("f2fs_victim_entry",
sizeof(struct victim_entry));
- if (!victim_entry_slab)
- return -ENOMEM;
- return 0;
+ return victim_entry_slab ? 0 : -ENOMEM;
}
void f2fs_destroy_garbage_collection_cache(void)
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
On 2022/12/13 6:45, Jaegeuk Kim wrote:
On 12/12, Chao Yu wrote:
On 2022/12/12 22:14, Yangtao Li wrote:
Hi Chao,
The difference here is, if we use f2fs_realtime_discard_enable() in
f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag
when discard option is enable and device supports discard.
But actually, if discard option is disabled, we still needs to give
put_super() a chance to write checkpoint w/ CP_TRIMMED flag.
Why do we still have to set the CP_TRIMMED flag when the discard opt is not set.
Did I miss something?
Hi Yangtao,
I guess it's up to scenario. e.g.
mount w/ nodiscard and use FITRIM to trigger in-batch discard,
if we set CP_TRIMMED flag during umount, next time, after mount
w/ discard, it doesn't to issue redundant discard.
If fitrim was called with a range, we can get a wrong FI_TRIMMED flag. Isn't it
We can set CP_TRIMMED flag only if fitrim was called on full range w/ 4k
granularity,
due to it will check sbi->discard_blks variable to make sure there is no range
we
haven't trimmed.
better to get a full discard range after remount even though some are redundant?
If nodiscard is set, and sbi->discard_blks becomes zero, it says a full range
fitrim
was been triggered.
So, previous check condition has no problem, right?
if ((f2fs_hw_support_discard(sbi) || f2fs_hw_should_discard(sbi)) &&
!sbi->discard_blks && !dropped) {
Thanks,
Thanks,
Thx,
Yangtao
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 12/15] f2fs: remove the create argument to f2fs_map_blocks
Hi Christoph,
On 11/28, Christoph Hellwig wrote:
> The create argument is always identicaly to map->m_may_create, so use
> that consistently.
>
> Signed-off-by: Christoph Hellwig
> ---
> fs/f2fs/data.c | 32 ++--
> fs/f2fs/f2fs.h | 3 +--
> fs/f2fs/file.c | 12 ++--
> include/trace/events/f2fs.h | 11 ---
> 4 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 2ae8fcf7cf49f4..730b58ba97c0ae 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1454,8 +1454,7 @@ int f2fs_get_block_locked(struct dnode_of_data *dn,
> pgoff_t index)
> * maps continuous logical blocks to physical blocks, and return such
> * info via f2fs_map_blocks structure.
> */
> -int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> - int create, int flag)
> +int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int
> flag)
> {
> unsigned int maxblocks = map->m_len;
> struct dnode_of_data dn;
> @@ -1484,11 +1483,7 @@ int f2fs_map_blocks(struct inode *inode, struct
> f2fs_map_blocks *map,
> pgofs = (pgoff_t)map->m_lblk;
> end = pgofs + maxblocks;
>
> - if (!create && f2fs_lookup_extent_cache(inode, pgofs, )) {
> - if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
Any reason to remove this condition?
Thanks,
> - map->m_may_create)
> - goto next_dnode;
> -
> + if (!map->m_may_create && f2fs_lookup_extent_cache(inode, pgofs, )) {
> map->m_pblk = ei.blk + pgofs - ei.fofs;
> map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs);
> map->m_flags = F2FS_MAP_MAPPED;
> @@ -1501,18 +1496,12 @@ int f2fs_map_blocks(struct inode *inode, struct
> f2fs_map_blocks *map,
> map->m_pblk, map->m_len);
>
> if (map->m_multidev_dio) {
> - block_t blk_addr = map->m_pblk;
> -
> bidx = f2fs_target_device_index(sbi, map->m_pblk);
>
> map->m_bdev = FDEV(bidx).bdev;
> map->m_pblk -= FDEV(bidx).start_blk;
> map->m_len = min(map->m_len,
> FDEV(bidx).end_blk + 1 - map->m_pblk);
> -
> - if (map->m_may_create)
> - f2fs_update_device_state(sbi, inode->i_ino,
> - blk_addr, map->m_len);
> }
> goto out;
> }
> @@ -1579,7 +1568,7 @@ int f2fs_map_blocks(struct inode *inode, struct
> f2fs_map_blocks *map,
> set_inode_flag(inode, FI_APPEND_WRITE);
> }
> } else {
> - if (create) {
> + if (map->m_may_create) {
> if (unlikely(f2fs_cp_error(sbi))) {
> err = -EIO;
> goto sync_out;
> @@ -1753,7 +1742,7 @@ int f2fs_map_blocks(struct inode *inode, struct
> f2fs_map_blocks *map,
> f2fs_balance_fs(sbi, dn.node_changed);
> }
> out:
> - trace_f2fs_map_blocks(inode, map, create, flag, err);
> + trace_f2fs_map_blocks(inode, map, flag, err);
> return err;
> }
>
> @@ -1775,7 +1764,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t pos,
> size_t len)
>
> while (map.m_lblk < last_lblk) {
> map.m_len = last_lblk - map.m_lblk;
> - err = f2fs_map_blocks(inode, , 0, F2FS_GET_BLOCK_DEFAULT);
> + err = f2fs_map_blocks(inode, , F2FS_GET_BLOCK_DEFAULT);
> if (err || map.m_len == 0)
> return false;
> map.m_lblk += map.m_len;
> @@ -1949,7 +1938,7 @@ int f2fs_fiemap(struct inode *inode, struct
> fiemap_extent_info *fieinfo,
> map.m_len = cluster_size - count_in_cluster;
> }
>
> - ret = f2fs_map_blocks(inode, , 0, F2FS_GET_BLOCK_FIEMAP);
> + ret = f2fs_map_blocks(inode, , F2FS_GET_BLOCK_FIEMAP);
> if (ret)
> goto out;
>
> @@ -2082,7 +2071,7 @@ static int f2fs_read_single_page(struct inode *inode,
> struct page *page,
> map->m_lblk = block_in_file;
> map->m_len = last_block - block_in_file;
>
> - ret = f2fs_map_blocks(inode, map, 0, F2FS_GET_BLOCK_DEFAULT);
> + ret = f2fs_map_blocks(inode, map, F2FS_GET_BLOCK_DEFAULT);
> if (ret)
> goto out;
> got_it:
> @@ -3779,7 +3768,7 @@ static sector_t f2fs_bmap(struct address_space
> *mapping, sector_t block)
> map.m_next_pgofs = NULL;
> map.m_seg_type = NO_CHECK_TYPE;
>
> - if (!f2fs_map_blocks(inode, , 0, F2FS_GET_BLOCK_BMAP))
> + if (!f2fs_map_blocks(inode, , F2FS_GET_BLOCK_BMAP))
>
Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
On 2022/12/13 6:47, Jaegeuk Kim wrote:
On 12/12, Chao Yu wrote:
On 2022/12/12 20:51, Yangtao Li wrote:
This patch adds support for counting the average time and
peak time of submit discard command, and we can see its
value in debugfs.
It is not sure whether the block layer has recorded these
data, and these data are allowed to be accessed by fs,
or they are only exported to user space.
On the one hand, I added these data to better understand
the current device operating status, and to further control
the discard process in a more detailed manner based on the
discard submit time in the future.
Again, w'd better to consider this functionality only when DEBUG_FS is
enabled.
BTW, why can't we use iostat to get the discard latencies?
Agreed.
Thanks,
Signed-off-by: Yangtao Li
---
fs/f2fs/debug.c | 10 +++---
fs/f2fs/f2fs.h| 6 ++
fs/f2fs/segment.c | 21 +++--
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 32af4f0c5735..142c256b89d9 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
llist_empty(_I(sbi)->fcc_info->issue_list);
}
if (SM_I(sbi)->dcc_info) {
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+ si->discard_avg = dcc->discard_time_avg;
+ si->discard_peak = dcc->discard_time_peak;
si->nr_discarded =
atomic_read(_I(sbi)->dcc_info->issued_discard);
si->nr_discarding =
@@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
si->nr_wb_cp_data, si->nr_wb_data,
si->nr_flushing, si->nr_flushed,
si->flush_list_empty);
- seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
- si->nr_discarding, si->nr_discarded,
- si->nr_discard_cmd, si->undiscard_blks);
+ seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd:
%4d undiscard:%4u\n",
+ si->nr_discarding, si->nr_discarded,
ktime_to_us(si->discard_avg),
+ ktime_to_us(si->discard_peak), si->nr_discard_cmd,
si->undiscard_blks);
seq_printf(s, " - atomic IO: %4d (Max. %4d)\n",
si->aw_cnt, si->max_aw_cnt);
seq_printf(s, " - compress: %4d, hit:%8d\n", si->compress_pages,
si->compress_page_hit);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..2cd55cb981ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -371,6 +371,8 @@ struct discard_cmd {
int error; /* bio error */
spinlock_t lock;/* for state/bio_ref updating */
unsigned short bio_ref; /* bio reference count */
+ struct discard_cmd_control *dcc;/* global discard cmd control */
+ ktime_t submit_start; /* submit start time */
};
enum {
@@ -415,6 +417,9 @@ struct discard_cmd_control {
unsigned int max_ordered_discard; /* maximum discard granularity
issued by lba order */
unsigned int undiscard_blks;/* # of undiscard blocks */
unsigned int next_pos; /* next discard position */
+ spinlock_t discard_time_lock; /* for discard time statistics */
+ ktime_t discard_time_avg; /* issued discard cmd avg time
*/
+ ktime_t discard_time_peak; /* issued discard cmd peak time
*/
atomic_t issued_discard;/* # of issued discard */
atomic_t queued_discard;/* # of queued discard */
atomic_t discard_cmd_cnt; /* # of cached cmd count */
@@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
int nr_dio_read, nr_dio_write;
unsigned int io_skip_bggc, other_skip_bggc;
int nr_flushing, nr_flushed, flush_list_empty;
+ ktime_t discard_avg, discard_peak;
int nr_discarding, nr_discarded;
int nr_discard_cmd;
unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..73cd05bb3f4a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct
f2fs_sb_info *sbi,
list_add_tail(>list, pend_list);
spin_lock_init(>lock);
dc->bio_ref = 0;
+ dc->dcc = dcc;
atomic_inc(>discard_cmd_cnt);
dcc->undiscard_blks += len;
@@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info
*sbi,
static void f2fs_submit_discard_endio(struct bio *bio)
{
struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+ struct discard_cmd_control *dcc = dc->dcc;
unsigned long flags;
+ ktime_t submit_time;
+
Re: [f2fs-dev] [PATCH] f2fs: don't set FI_COMPRESS_RELEASED if file is not compressed
On 12/12, zhoudan wrote: > Maybe I'm not describing it clearly enough, but I think there is > something wrong with the logic here.The 'f2fs_release_compress_blocks' > method does not determine if the file is compressed, but simply adds > the FI_COMPRESS_RELEASED flag. I firstly lost your point since f2fs_release_compress_blocks() checked f2fs_compressed_file(). > In particular, in the current Android system, when the application is > installed, the release interface is called by default to release the > storage marked as compressed, without checking whether the file is > actually compressed. In this case, when compress_mode is set to user, > calling the compress interface returns ENVAL and the file cannot be > compressed. > So I think the implementation of release needs to be modified, and > only set FI_COMPRESS_RELEASED when it's really compressed and the > storage is released. > > On Fri, Dec 09, 2022 at 12:19:44PM -0800, Jaegeuk Kim wrote: > > On 12/08, zhoudan8 wrote: > > > In compress_mode=user, f2fs_release_compress_blocks() > > > does not verify whether it has been compressed and > > > sets FI_COMPRESS_RELEASED directly. which will lead to > > > return -EINVAL after calling compress. > > > To fix it,let's do not set FI_COMPRESS_RELEASED if file > > > is not compressed. > > > > Do you mean you want to avoid EINVAL on a file having FI_COMPRESS_RELEASED > > with zero i_compr_blokcs? > > > > I think the current logic is giving the error on a released file already. > > > > > > > > Signed-off-by: zhoudan8 > > > --- > > > fs/f2fs/file.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > > > index 82cda1258227..f32910077df6 100644 > > > --- a/fs/f2fs/file.c > > > +++ b/fs/f2fs/file.c > > > @@ -3451,14 +3451,13 @@ static int f2fs_release_compress_blocks(struct > > > file *filp, unsigned long arg) > > > ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); > > > if (ret) > > > goto out; > > > - > > > - set_inode_flag(inode, FI_COMPRESS_RELEASED); > > > inode->i_ctime = current_time(inode); > > > f2fs_mark_inode_dirty_sync(inode, true); > > > > > > if (!atomic_read(_I(inode)->i_compr_blocks)) > > > goto out; > > > > > > + set_inode_flag(inode, FI_COMPRESS_RELEASED); > > > f2fs_down_write(_I(inode)->i_gc_rwsem[WRITE]); > > > filemap_invalidate_lock(inode->i_mapping); > > > > > > -- > > > 2.38.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: add iostat support for FS_DISCARD_IO
Please check the previous comment. On 12/12, Chao Yu wrote: > On 2022/12/12 13:47, Yangtao Li wrote: > > Just like other data we count uses the number of bytes as the basic > > unit, but discard uses the number of cmds as the statistical unit. In > > fact, the discard command contains the number of blocks. In order to > > avoid breaking its usage of application, let's keeping FS_DISCARD > > as it is, and add FS_DISCARD_IO to account discard bytes. > > > > Suggested-by: Chao Yu > > Signed-off-by: Yangtao Li > > Reviewed-by: Chao Yu > > Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix iostat parameter for discard
On 12/11, Chao Yu wrote:
> On 2022/12/5 22:56, Yangtao Li wrote:
> > Just like other data we count uses the number of bytes as the basic unit,
> > but discard uses the number of cmds as the statistical unit. In fact the
> > discard command contains the number of blocks, so let's change to the
> > number of bytes as the base unit.
> >
> > Fixes: b0af6d491a6b ("f2fs: add app/fs io stat")
> >
> > Signed-off-by: Yangtao Li
> > ---
> > fs/f2fs/segment.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 9486ca49ecb1..bc262e17b279 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1181,7 +1181,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info
> > *sbi,
> > atomic_inc(>issued_discard);
> > - f2fs_update_iostat(sbi, NULL, FS_DISCARD, 1);
> > + f2fs_update_iostat(sbi, NULL, FS_DISCARD, len * F2FS_BLKSIZE);
>
> In order to avoid breaking its usage of application, how about keeping
> FS_DISCARD as it is, and add FS_DISCARD_IO to account discard bytes?
I picked this to match the f2fs_update_iostat() definition. Do we know
how many applications will be affected? I don't have any at a glance in
Android tho.
void f2fs_update_iostat(struct f2fs_sb_info *sbi, struct inode *inode,
enum iostat_type type, unsigned long long io_bytes)
>
> Thanks,
>
> > lstart += len;
> > start += len;
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: do some cleanup for f2fs module init
On 12/11, Chao Yu wrote:
> On 2022/11/25 19:47, Yangtao Li wrote:
> > Just for cleanup, no functional changes.
> >
> > Signed-off-by: Yangtao Li
> > ---
> > fs/f2fs/compress.c | 46 ++
> > fs/f2fs/data.c | 14 --
> > fs/f2fs/gc.c | 4 +---
> > fs/f2fs/recovery.c | 4 +---
> > fs/f2fs/super.c| 8 ++--
> > 5 files changed, 14 insertions(+), 62 deletions(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index d315c2de136f..f920ba8e0e85 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -567,10 +567,7 @@ MODULE_PARM_DESC(num_compress_pages,
> > int f2fs_init_compress_mempool(void)
> > {
> > compress_page_pool = mempool_create_page_pool(num_compress_pages, 0);
> > - if (!compress_page_pool)
> > - return -ENOMEM;
> > -
> > - return 0;
> > + return compress_page_pool ? 0 : -ENOMEM;
>
> I don't think this needs cleanup, other part looks good to me.
What is the point here comparing to the below? fyi; I picked this change.
>
> Thanks,
>
> > }
> > void f2fs_destroy_compress_mempool(void)
> > @@ -1983,9 +1980,7 @@ int f2fs_init_page_array_cache(struct f2fs_sb_info
> > *sbi)
> > sbi->page_array_slab = f2fs_kmem_cache_create(slab_name,
> > sbi->page_array_slab_size);
> > - if (!sbi->page_array_slab)
> > - return -ENOMEM;
> > - return 0;
> > + return sbi->page_array_slab ? 0 : -ENOMEM;
> > }
> > void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi)
> > @@ -1993,53 +1988,24 @@ void f2fs_destroy_page_array_cache(struct
> > f2fs_sb_info *sbi)
> > kmem_cache_destroy(sbi->page_array_slab);
> > }
> > -static int __init f2fs_init_cic_cache(void)
> > +int __init f2fs_init_compress_cache(void)
> > {
> > cic_entry_slab = f2fs_kmem_cache_create("f2fs_cic_entry",
> > sizeof(struct compress_io_ctx));
> > if (!cic_entry_slab)
> > return -ENOMEM;
> > - return 0;
> > -}
> > -
> > -static void f2fs_destroy_cic_cache(void)
> > -{
> > - kmem_cache_destroy(cic_entry_slab);
> > -}
> > -
> > -static int __init f2fs_init_dic_cache(void)
> > -{
> > dic_entry_slab = f2fs_kmem_cache_create("f2fs_dic_entry",
> > sizeof(struct decompress_io_ctx));
> > if (!dic_entry_slab)
> > - return -ENOMEM;
> > - return 0;
> > -}
> > -
> > -static void f2fs_destroy_dic_cache(void)
> > -{
> > - kmem_cache_destroy(dic_entry_slab);
> > -}
> > -
> > -int __init f2fs_init_compress_cache(void)
> > -{
> > - int err;
> > -
> > - err = f2fs_init_cic_cache();
> > - if (err)
> > - goto out;
> > - err = f2fs_init_dic_cache();
> > - if (err)
> > goto free_cic;
> > return 0;
> > free_cic:
> > - f2fs_destroy_cic_cache();
> > -out:
> > + kmem_cache_destroy(cic_entry_slab);
> > return -ENOMEM;
> > }
> > void f2fs_destroy_compress_cache(void)
> > {
> > - f2fs_destroy_dic_cache();
> > - f2fs_destroy_cic_cache();
> > + kmem_cache_destroy(dic_entry_slab);
> > + kmem_cache_destroy(cic_entry_slab);
> > }
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 560fa80590e9..35c19248b1e2 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -39,10 +39,8 @@ static struct bio_set f2fs_bioset;
> > int __init f2fs_init_bioset(void)
> > {
> > - if (bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
> > - 0, BIOSET_NEED_BVECS))
> > - return -ENOMEM;
> > - return 0;
> > + return bioset_init(_bioset, F2FS_BIO_POOL_SIZE,
> > + 0, BIOSET_NEED_BVECS);
> > }
> > void f2fs_destroy_bioset(void)
> > @@ -4090,9 +4088,7 @@ int f2fs_init_post_read_wq(struct f2fs_sb_info *sbi)
> > sbi->post_read_wq = alloc_workqueue("f2fs_post_read_wq",
> > WQ_UNBOUND | WQ_HIGHPRI,
> > num_online_cpus());
> > - if (!sbi->post_read_wq)
> > - return -ENOMEM;
> > - return 0;
> > + return sbi->post_read_wq ? 0 : -ENOMEM;
> > }
> > void f2fs_destroy_post_read_wq(struct f2fs_sb_info *sbi)
> > @@ -4105,9 +4101,7 @@ int __init f2fs_init_bio_entry_cache(void)
> > {
> > bio_entry_slab = f2fs_kmem_cache_create("f2fs_bio_entry_slab",
> > sizeof(struct bio_entry));
> > - if (!bio_entry_slab)
> > - return -ENOMEM;
> > - return 0;
> > + return bio_entry_slab ? 0 : -ENOMEM;
> > }
> > void f2fs_destroy_bio_entry_cache(void)
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 0f967b1e98f2..4b0d2fa3a769 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -1903,9 +1903,7 @@ int __init f2fs_create_garbage_collection_cache(void)
> > {
> > victim_entry_slab = f2fs_kmem_cache_create("f2fs_victim_entry",
> > sizeof(struct
Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
On 12/12, Chao Yu wrote:
> On 2022/12/12 20:51, Yangtao Li wrote:
> > This patch adds support for counting the average time and
> > peak time of submit discard command, and we can see its
> > value in debugfs.
> >
> > It is not sure whether the block layer has recorded these
> > data, and these data are allowed to be accessed by fs,
> > or they are only exported to user space.
> >
> > On the one hand, I added these data to better understand
> > the current device operating status, and to further control
> > the discard process in a more detailed manner based on the
> > discard submit time in the future.
>
> Again, w'd better to consider this functionality only when DEBUG_FS is
> enabled.
BTW, why can't we use iostat to get the discard latencies?
>
> >
> > Signed-off-by: Yangtao Li
> > ---
> > fs/f2fs/debug.c | 10 +++---
> > fs/f2fs/f2fs.h| 6 ++
> > fs/f2fs/segment.c | 21 +++--
> > 3 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index 32af4f0c5735..142c256b89d9 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info
> > *sbi)
> > llist_empty(_I(sbi)->fcc_info->issue_list);
> > }
> > if (SM_I(sbi)->dcc_info) {
> > + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> > +
> > + si->discard_avg = dcc->discard_time_avg;
> > + si->discard_peak = dcc->discard_time_peak;
> > si->nr_discarded =
> > atomic_read(_I(sbi)->dcc_info->issued_discard);
> > si->nr_discarding =
> > @@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
> >si->nr_wb_cp_data, si->nr_wb_data,
> >si->nr_flushing, si->nr_flushed,
> >si->flush_list_empty);
> > - seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
> > - si->nr_discarding, si->nr_discarded,
> > - si->nr_discard_cmd, si->undiscard_blks);
> > + seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns))
> > cmd: %4d undiscard:%4u\n",
> > + si->nr_discarding, si->nr_discarded,
> > ktime_to_us(si->discard_avg),
> > + ktime_to_us(si->discard_peak), si->nr_discard_cmd,
> > si->undiscard_blks);
> > seq_printf(s, " - atomic IO: %4d (Max. %4d)\n",
> >si->aw_cnt, si->max_aw_cnt);
> > seq_printf(s, " - compress: %4d, hit:%8d\n",
> > si->compress_pages, si->compress_page_hit);
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index e8953c3dc81a..2cd55cb981ff 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -371,6 +371,8 @@ struct discard_cmd {
> > int error; /* bio error */
> > spinlock_t lock;/* for state/bio_ref updating */
> > unsigned short bio_ref; /* bio reference count */
> > + struct discard_cmd_control *dcc;/* global discard cmd control */
> > + ktime_t submit_start; /* submit start time */
> > };
> > enum {
> > @@ -415,6 +417,9 @@ struct discard_cmd_control {
> > unsigned int max_ordered_discard; /* maximum discard granularity
> > issued by lba order */
> > unsigned int undiscard_blks;/* # of undiscard blocks */
> > unsigned int next_pos; /* next discard position */
> > + spinlock_t discard_time_lock; /* for discard time statistics */
> > + ktime_t discard_time_avg; /* issued discard cmd avg time
> > */
> > + ktime_t discard_time_peak; /* issued discard cmd peak time
> > */
> > atomic_t issued_discard;/* # of issued discard */
> > atomic_t queued_discard;/* # of queued discard */
> > atomic_t discard_cmd_cnt; /* # of cached cmd count */
> > @@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
> > int nr_dio_read, nr_dio_write;
> > unsigned int io_skip_bggc, other_skip_bggc;
> > int nr_flushing, nr_flushed, flush_list_empty;
> > + ktime_t discard_avg, discard_peak;
> > int nr_discarding, nr_discarded;
> > int nr_discard_cmd;
> > unsigned int undiscard_blks;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index a9099a754dd2..73cd05bb3f4a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct
> > f2fs_sb_info *sbi,
> > list_add_tail(>list, pend_list);
> > spin_lock_init(>lock);
> > dc->bio_ref = 0;
> > + dc->dcc = dcc;
> > atomic_inc(>discard_cmd_cnt);
> > dcc->undiscard_blks += len;
> > @@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info
> > *sbi,
> > static void f2fs_submit_discard_endio(struct bio *bio)
> > {
> > struct discard_cmd *dc
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
On 12/12, Chao Yu wrote: > On 2022/12/12 22:14, Yangtao Li wrote: > > Hi Chao, > > > > > The difference here is, if we use f2fs_realtime_discard_enable() in > > > f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag > > > when discard option is enable and device supports discard. > > > > > But actually, if discard option is disabled, we still needs to give > > > put_super() a chance to write checkpoint w/ CP_TRIMMED flag. > > > > Why do we still have to set the CP_TRIMMED flag when the discard opt is not > > set. > > Did I miss something? > > Hi Yangtao, > > I guess it's up to scenario. e.g. > > mount w/ nodiscard and use FITRIM to trigger in-batch discard, > if we set CP_TRIMMED flag during umount, next time, after mount > w/ discard, it doesn't to issue redundant discard. If fitrim was called with a range, we can get a wrong FI_TRIMMED flag. Isn't it better to get a full discard range after remount even though some are redundant? > > Thanks, > > > > > Thx, > > Yangtao ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [Bug 216050] f2fs_gc occupies 100% cpu
https://bugzilla.kernel.org/show_bug.cgi?id=216050 --- Comment #110 from Guido (guido.iod...@gmail.com) --- I deactivate the f2fs-gc script for two days and... again the 100% cpu on f2fs_gc process :-( -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [RFC PATCH] f2fs: Convert f2fs_write_cache_pages() to use filemap_get_folios_tag()
Converted the function to use a folio_batch instead of pagevec. This is in
preparation for the removal of find_get_pages_range_tag().
Also modified f2fs_all_cluster_page_ready to take in a folio_batch instead
of pagevec. This does NOT support large folios. The function currently
only utilizes folios of size 1 so this shouldn't cause any issues right
now.
This version of the patch limits the number of pages fetched to
F2FS_ONSTACK_PAGES. If that ever happens, update the start index here
since filemap_get_folios_tag() updates the index to be after the last
found folio, not necessarily the last used page.
Signed-off-by: Vishal Moola (Oracle)
---
Let me know if you prefer this version and I'll include it in v5
of the patch series when I rebase it after the merge window.
---
fs/f2fs/data.c | 86 ++
1 file changed, 59 insertions(+), 27 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a71e818cd67b..1703e353f0e0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2939,6 +2939,7 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
int ret = 0;
int done = 0, retry = 0;
struct page *pages[F2FS_ONSTACK_PAGES];
+ struct folio_batch fbatch;
struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
struct bio *bio = NULL;
sector_t last_block;
@@ -2959,6 +2960,7 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
.private = NULL,
};
#endif
+ int nr_folios, p, idx;
int nr_pages;
pgoff_t index;
pgoff_t end;/* Inclusive */
@@ -2969,6 +2971,8 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
int submitted = 0;
int i;
+ folio_batch_init();
+
if (get_dirty_pages(mapping->host) <=
SM_I(F2FS_M_SB(mapping))->min_hot_blocks)
set_inode_flag(mapping->host, FI_HOT_DATA);
@@ -2994,13 +2998,38 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
tag_pages_for_writeback(mapping, index, end);
done_index = index;
while (!done && !retry && (index <= end)) {
- nr_pages = find_get_pages_range_tag(mapping, , end,
- tag, F2FS_ONSTACK_PAGES, pages);
- if (nr_pages == 0)
+ nr_pages = 0;
+again:
+ nr_folios = filemap_get_folios_tag(mapping, , end,
+ tag, );
+ if (nr_folios == 0) {
+ if (nr_pages)
+ goto write;
break;
+ }
+ for (i = 0; i < nr_folios; i++) {
+ struct folio* folio = fbatch.folios[i];
+
+ idx = 0;
+ p = folio_nr_pages(folio);
+add_more:
+ pages[nr_pages] = folio_page(folio,idx);
+ folio_ref_inc(folio);
+ if (++nr_pages == F2FS_ONSTACK_PAGES) {
+ index = folio->index + idx + 1;
+ folio_batch_release();
+ goto write;
+ }
+ if (++idx < p)
+ goto add_more;
+ }
+ folio_batch_release();
+ goto again;
+write:
for (i = 0; i < nr_pages; i++) {
struct page *page = pages[i];
+ struct folio *folio = page_folio(page);
bool need_readd;
readd:
need_readd = false;
@@ -3017,7 +3046,7 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
}
if (!f2fs_cluster_can_merge_page(,
- page->index)) {
+ folio->index)) {
ret = f2fs_write_multi_pages(,
, wbc, io_type);
if (!ret)
@@ -3026,27 +3055,28 @@ static int f2fs_write_cache_pages(struct address_space
*mapping,
}
if (unlikely(f2fs_cp_error(sbi)))
- goto lock_page;
+ goto lock_folio;
if (!f2fs_cluster_is_empty())
- goto lock_page;
+ goto lock_folio;
if (f2fs_all_cluster_page_ready(,
pages, i, nr_pages, true))
- goto lock_page;
+ goto lock_folio;
Re: [f2fs-dev] [PATCH v3 14/23] f2fs: Convert f2fs_write_cache_pages() to use filemap_get_folios_tag()
Hi Vishal,
Sorry for the delay reply.
On 2022/12/6 4:34, Vishal Moola wrote:
On Tue, Nov 22, 2022 at 6:26 PM Vishal Moola wrote:
On Mon, Nov 14, 2022 at 1:38 PM Vishal Moola wrote:
On Sun, Nov 13, 2022 at 11:02 PM Chao Yu wrote:
On 2022/10/18 4:24, Vishal Moola (Oracle) wrote:
Converted the function to use a folio_batch instead of pagevec. This is in
preparation for the removal of find_get_pages_range_tag().
Also modified f2fs_all_cluster_page_ready to take in a folio_batch instead
of pagevec. This does NOT support large folios. The function currently
Vishal,
It looks this patch tries to revert Fengnan's change:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=01fc4b9a6ed8eacb64e5609bab7ac963e1c7e486
How about doing some tests to evaluate its performance effect?
Yeah I'll play around with it to see how much of a difference it makes.
I did some testing. Looks like reverting Fengnan's change allows for
occasional, but significant, spikes in write latency. I'll work on a variation
of the patch that maintains the use of F2FS_ONSTACK_PAGES and send
that in the next version of the patch series. Thanks for pointing that out!
Following Matthew's comment, I'm thinking we should go with this patch
as is. The numbers between both variations did not have substantial
differences with regard to latency.
While the new variant would maintain the use of F2FS_ONSTACK_PAGES,
the code becomes messier and would end up limiting the number of
folios written back once large folio support is added. This means it would
have to be converted down to this version later anyways.
Does leaving this patch as is sound good to you?
For reference, here's what the version continuing to use a page
array of size F2FS_ONSTACK_PAGES would change:
+ nr_pages = 0;
+again:
+ nr_folios = filemap_get_folios_tag(mapping, , end,
+ tag, );
+ if (nr_folios == 0) {
+ if (nr_pages)
+ goto write;
+ goto write;
Duplicated code.
break;
+ }
+ for (i = 0; i < nr_folios; i++) {
+ struct folio* folio = fbatch.folios[i];
+
+ idx = 0;
+ p = folio_nr_pages(folio);
+add_more:
+ pages[nr_pages] = folio_page(folio,idx);
+ folio_ref_inc(folio);
+ if (++nr_pages == F2FS_ONSTACK_PAGES) {
+ index = folio->index + idx + 1;
+ folio_batch_release();
+ goto write;
+ }
+ if (++idx < p)
+ goto add_more;
+ }
+ folio_batch_release();
+ goto again;
+write:
Looks fine to me, can you please send a formal patch?
Thanks,
How do the remaining f2fs patches in the series look to you?
Patch 16/23 f2fs_sync_meta_pages() in particular seems like it may
be prone to problems. If there are any changes that need to be made to
it I can include those in the next version as well.
Thanks for reviewing the patches so far. I wanted to follow up on asking
for review of the last couple of patches.
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
On 2022/12/12 22:14, Yangtao Li wrote: Hi Chao, The difference here is, if we use f2fs_realtime_discard_enable() in f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag when discard option is enable and device supports discard. But actually, if discard option is disabled, we still needs to give put_super() a chance to write checkpoint w/ CP_TRIMMED flag. Why do we still have to set the CP_TRIMMED flag when the discard opt is not set. Did I miss something? Hi Yangtao, I guess it's up to scenario. e.g. mount w/ nodiscard and use FITRIM to trigger in-batch discard, if we set CP_TRIMMED flag during umount, next time, after mount w/ discard, it doesn't to issue redundant discard. Thanks, Thx, Yangtao ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
Hi Chao, > The difference here is, if we use f2fs_realtime_discard_enable() in > f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag > when discard option is enable and device supports discard. > But actually, if discard option is disabled, we still needs to give > put_super() a chance to write checkpoint w/ CP_TRIMMED flag. Why do we still have to set the CP_TRIMMED flag when the discard opt is not set. Did I miss something? Thx, Yangtao ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v2] f2fs: introduce discard_io_aware_gran sysfs node
The current discard_io_aware_gran is a fixed value, change it to be
configurable through the sys node.
Signed-off-by: Yangtao Li
---
v2:
- allow 0
Documentation/ABI/testing/sysfs-fs-f2fs | 9 +
fs/f2fs/f2fs.h | 3 +++
fs/f2fs/segment.c | 3 ++-
fs/f2fs/sysfs.c | 13 +
4 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs
b/Documentation/ABI/testing/sysfs-fs-f2fs
index 9e3756625a81..7b6cd4cf40ce 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -669,3 +669,12 @@ Contact: "Ping Xiong"
Description: When DATA SEPARATION is on, it controls the age threshold to
indicate
the data blocks as warm. By default it was initialized as
2621440 blocks
(equals to 10GB).
+
+What: /sys/fs/f2fs//discard_io_aware_gran
+Date: December 2022
+Contact: "Yangtao Li"
+Description: Controls background discard granularity of inner discard thread
+ when is not in idle. Inner thread will not issue discards with
size that
+ is smaller than granularity. The unit size is one block(4KB),
now only
+ support configuring in range of [0, 512].
+ Default: 512
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..bd1430d09c6d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -332,6 +332,8 @@ struct discard_entry {
#define DEFAULT_DISCARD_GRANULARITY16
/* default maximum discard granularity of ordered discard, unit: block count */
#define DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY16
+/* default minimum granularity discard not be aware of I/O, unit: block count
*/
+#define DEFAULT_IO_AWARE_DISCARD_GRANULARITY 512
/* max discard pend list number */
#define MAX_PLIST_NUM 512
@@ -410,6 +412,7 @@ struct discard_cmd_control {
unsigned int min_discard_issue_time;/* min. interval between
discard issue */
unsigned int mid_discard_issue_time;/* mid. interval between
discard issue */
unsigned int max_discard_issue_time;/* max. interval between
discard issue */
+ unsigned int discard_io_aware_gran; /* minimum discard granularity not
be aware of I/O */
unsigned int discard_urgent_util; /* utilization which issue
discard proactively */
unsigned int discard_granularity; /* discard granularity */
unsigned int max_ordered_discard; /* maximum discard granularity
issued by lba order */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..f4bf39ee31c6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1060,7 +1060,7 @@ static void __init_discard_policy(struct f2fs_sb_info
*sbi,
dpolicy->granularity = granularity;
dpolicy->max_requests = dcc->max_discard_request;
- dpolicy->io_aware_gran = MAX_PLIST_NUM;
+ dpolicy->io_aware_gran = dcc->discard_io_aware_gran;
dpolicy->timeout = false;
if (discard_type == DPOLICY_BG) {
@@ -2066,6 +2066,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info
*sbi)
if (!dcc)
return -ENOMEM;
+ dcc->discard_io_aware_gran = DEFAULT_IO_AWARE_DISCARD_GRANULARITY;
dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY;
dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY;
if (F2FS_OPTION(sbi).discard_unit == DISCARD_UNIT_SEGMENT)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 83a366f3ee80..5ab42da5f2a3 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -473,6 +473,17 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
return count;
}
+ if (!strcmp(a->attr.name, "discard_io_aware_gran")) {
+ if (t > MAX_PLIST_NUM)
+ return -EINVAL;
+ if (!f2fs_block_unit_discard(sbi))
+ return -EINVAL;
+ if (t == *ui)
+ return count;
+ *ui = t;
+ return count;
+ }
+
if (!strcmp(a->attr.name, "discard_granularity")) {
if (t == 0 || t > MAX_PLIST_NUM)
return -EINVAL;
@@ -825,6 +836,7 @@ F2FS_RW_ATTR(DCC_INFO, discard_cmd_control,
max_discard_request, max_discard_req
F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, min_discard_issue_time,
min_discard_issue_time);
F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, mid_discard_issue_time,
mid_discard_issue_time);
F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, max_discard_issue_time,
max_discard_issue_time);
+F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_io_aware_gran,
discard_io_aware_gran);
F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_urgent_util,
discard_urgent_util);
F2FS_RW_ATTR(DCC_INFO, discard_cmd_control, discard_granularity,
discard_granularity);
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
On 2022/12/12 21:05, Yangtao Li wrote:
Hi,
static inline bool f2fs_realtime_discard_enable(struct f2fs_sb_info *sbi) {
return (test_opt(sbi, DISCARD) && f2fs_hw_support_discard(sbi)) ||
f2fs_hw_should_discard(sbi);
}
It looks the logic is changed?
For a storage device that does not support discard, and we have not actually
issued any discard command. I don't think it is necessary and f2fs should not
be equipped with trim markers.
The difference here is, if we use f2fs_realtime_discard_enable() in
f2fs_put_super(), we will only write checkpoint w/ CP_TRIMMED flag
when discard option is enable and device supports discard.
But actually, if discard option is disabled, we still needs to give
put_super() a chance to write checkpoint w/ CP_TRIMMED flag.
Thanks,
Thx,
Yangtao
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
On 2022/12/12 20:51, Yangtao Li wrote:
This patch adds support for counting the average time and
peak time of submit discard command, and we can see its
value in debugfs.
It is not sure whether the block layer has recorded these
data, and these data are allowed to be accessed by fs,
or they are only exported to user space.
On the one hand, I added these data to better understand
the current device operating status, and to further control
the discard process in a more detailed manner based on the
discard submit time in the future.
Again, w'd better to consider this functionality only when DEBUG_FS is
enabled.
Signed-off-by: Yangtao Li
---
fs/f2fs/debug.c | 10 +++---
fs/f2fs/f2fs.h| 6 ++
fs/f2fs/segment.c | 21 +++--
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 32af4f0c5735..142c256b89d9 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
llist_empty(_I(sbi)->fcc_info->issue_list);
}
if (SM_I(sbi)->dcc_info) {
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+ si->discard_avg = dcc->discard_time_avg;
+ si->discard_peak = dcc->discard_time_peak;
si->nr_discarded =
atomic_read(_I(sbi)->dcc_info->issued_discard);
si->nr_discarding =
@@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
si->nr_wb_cp_data, si->nr_wb_data,
si->nr_flushing, si->nr_flushed,
si->flush_list_empty);
- seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
- si->nr_discarding, si->nr_discarded,
- si->nr_discard_cmd, si->undiscard_blks);
+ seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns)) cmd:
%4d undiscard:%4u\n",
+ si->nr_discarding, si->nr_discarded,
ktime_to_us(si->discard_avg),
+ ktime_to_us(si->discard_peak), si->nr_discard_cmd,
si->undiscard_blks);
seq_printf(s, " - atomic IO: %4d (Max. %4d)\n",
si->aw_cnt, si->max_aw_cnt);
seq_printf(s, " - compress: %4d, hit:%8d\n", si->compress_pages,
si->compress_page_hit);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..2cd55cb981ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -371,6 +371,8 @@ struct discard_cmd {
int error; /* bio error */
spinlock_t lock;/* for state/bio_ref updating */
unsigned short bio_ref; /* bio reference count */
+ struct discard_cmd_control *dcc;/* global discard cmd control */
+ ktime_t submit_start; /* submit start time */
};
enum {
@@ -415,6 +417,9 @@ struct discard_cmd_control {
unsigned int max_ordered_discard; /* maximum discard granularity
issued by lba order */
unsigned int undiscard_blks;/* # of undiscard blocks */
unsigned int next_pos; /* next discard position */
+ spinlock_t discard_time_lock; /* for discard time statistics */
+ ktime_t discard_time_avg; /* issued discard cmd avg time
*/
+ ktime_t discard_time_peak; /* issued discard cmd peak time
*/
atomic_t issued_discard;/* # of issued discard */
atomic_t queued_discard;/* # of queued discard */
atomic_t discard_cmd_cnt; /* # of cached cmd count */
@@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
int nr_dio_read, nr_dio_write;
unsigned int io_skip_bggc, other_skip_bggc;
int nr_flushing, nr_flushed, flush_list_empty;
+ ktime_t discard_avg, discard_peak;
int nr_discarding, nr_discarded;
int nr_discard_cmd;
unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..73cd05bb3f4a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct
f2fs_sb_info *sbi,
list_add_tail(>list, pend_list);
spin_lock_init(>lock);
dc->bio_ref = 0;
+ dc->dcc = dcc;
atomic_inc(>discard_cmd_cnt);
dcc->undiscard_blks += len;
@@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
static void f2fs_submit_discard_endio(struct bio *bio)
{
struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+ struct discard_cmd_control *dcc = dc->dcc;
unsigned long flags;
+ ktime_t submit_time;
+ int nr_discarded;
spin_lock_irqsave(>lock, flags);
+ submit_time = ktime_sub(ktime_get(), dc->submit_start);
if
[f2fs-dev] [PATCH] f2fs: convert discard_wake and gc_wake to bool type
discard_wake and gc_wake have only two values, 0 or 1.
So there is no need to use int type to store them.
BTW, move discard_wake to the end of the
discard_cmd_control structure.
Before:
- sizeof(struct discard_cmd_control): 8392
After move:
- sizeof(struct discard_cmd_control): 8384
Signed-off-by: Yangtao Li
---
fs/f2fs/f2fs.h| 2 +-
fs/f2fs/gc.c | 4 ++--
fs/f2fs/gc.h | 2 +-
fs/f2fs/segment.c | 2 +-
fs/f2fs/segment.h | 2 +-
fs/f2fs/sysfs.c | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..764041d7b217 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -402,7 +402,6 @@ struct discard_cmd_control {
struct list_head wait_list; /* store on-flushing entries */
struct list_head fstrim_list; /* in-flight discard from
fstrim */
wait_queue_head_t discard_wait_queue; /* waiting queue for wake-up */
- unsigned int discard_wake; /* to wake up discard thread */
struct mutex cmd_lock;
unsigned int nr_discards; /* # of discards in the list */
unsigned int max_discards; /* max. discards to be issued */
@@ -420,6 +419,7 @@ struct discard_cmd_control {
atomic_t discard_cmd_cnt; /* # of cached cmd count */
struct rb_root_cached root; /* root of discard rb-tree */
bool rbtree_check; /* config for consistence check
*/
+ bool discard_wake; /* to wake up discard thread */
};
/* for the list of fsync inodes, used only during recovery */
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index f0c6506d8975..678726e6a6e8 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -57,7 +57,7 @@ static int gc_thread_func(void *data)
/* give it a try one time */
if (gc_th->gc_wake)
- gc_th->gc_wake = 0;
+ gc_th->gc_wake = false;
if (try_to_freeze()) {
stat_other_skip_bggc_count(sbi);
@@ -181,7 +181,7 @@ int f2fs_start_gc_thread(struct f2fs_sb_info *sbi)
gc_th->max_sleep_time = DEF_GC_THREAD_MAX_SLEEP_TIME;
gc_th->no_gc_sleep_time = DEF_GC_THREAD_NOGC_SLEEP_TIME;
- gc_th->gc_wake = 0;
+ gc_th->gc_wake = false;
sbi->gc_thread = gc_th;
init_waitqueue_head(>gc_thread->gc_wait_queue_head);
diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h
index 19b956c2d697..15bd1d680f67 100644
--- a/fs/f2fs/gc.h
+++ b/fs/f2fs/gc.h
@@ -41,7 +41,7 @@ struct f2fs_gc_kthread {
unsigned int no_gc_sleep_time;
/* for changing gc mode */
- unsigned int gc_wake;
+ bool gc_wake;
/* for GC_MERGE mount option */
wait_queue_head_t fggc_wq; /*
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..a85d438e76ea 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1701,7 +1701,7 @@ static int issue_discard_thread(void *data)
dcc->discard_granularity);
if (dcc->discard_wake)
- dcc->discard_wake = 0;
+ dcc->discard_wake = false;
/* clean up pending candidates before going to sleep */
if (atomic_read(>queued_discard))
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 3ad1b7b6fa94..22fac8baf4a4 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -924,6 +924,6 @@ static inline void wake_up_discard_thread(struct
f2fs_sb_info *sbi, bool force)
if (!wakeup || !is_idle(sbi, DISCARD_TIME))
return;
wake_up:
- dcc->discard_wake = 1;
+ dcc->discard_wake = true;
wake_up_interruptible_all(>discard_wait_queue);
}
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 83a366f3ee80..805b632a3af0 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -511,7 +511,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
} else if (t == 1) {
sbi->gc_mode = GC_URGENT_HIGH;
if (sbi->gc_thread) {
- sbi->gc_thread->gc_wake = 1;
+ sbi->gc_thread->gc_wake = true;
wake_up_interruptible_all(
>gc_thread->gc_wait_queue_head);
wake_up_discard_thread(sbi, true);
@@ -521,7 +521,7 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
} else if (t == 3) {
sbi->gc_mode = GC_URGENT_MID;
if (sbi->gc_thread) {
- sbi->gc_thread->gc_wake = 1;
+ sbi->gc_thread->gc_wake = true;
wake_up_interruptible_all(
>gc_thread->gc_wait_queue_head);
}
--
Re: [f2fs-dev] [PATCH v2] f2fs: don't call f2fs_issue_discard_timeout() when discard_cmd_cnt is 0 in f2fs_put_super()
Hi,
> static inline bool f2fs_realtime_discard_enable(struct f2fs_sb_info *sbi) {
> return (test_opt(sbi, DISCARD) && f2fs_hw_support_discard(sbi)) ||
> f2fs_hw_should_discard(sbi);
> }
> It looks the logic is changed?
For a storage device that does not support discard, and we have not actually
issued any discard command. I don't think it is necessary and f2fs should not
be equipped with trim markers.
Thx,
Yangtao
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v2] f2fs: add support for counting time of submit discard cmd
This patch adds support for counting the average time and
peak time of submit discard command, and we can see its
value in debugfs.
It is not sure whether the block layer has recorded these
data, and these data are allowed to be accessed by fs,
or they are only exported to user space.
On the one hand, I added these data to better understand
the current device operating status, and to further control
the discard process in a more detailed manner based on the
discard submit time in the future.
Signed-off-by: Yangtao Li
---
fs/f2fs/debug.c | 10 +++---
fs/f2fs/f2fs.h| 6 ++
fs/f2fs/segment.c | 21 +++--
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 32af4f0c5735..142c256b89d9 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -120,6 +120,10 @@ static void update_general_status(struct f2fs_sb_info *sbi)
llist_empty(_I(sbi)->fcc_info->issue_list);
}
if (SM_I(sbi)->dcc_info) {
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+ si->discard_avg = dcc->discard_time_avg;
+ si->discard_peak = dcc->discard_time_peak;
si->nr_discarded =
atomic_read(_I(sbi)->dcc_info->issued_discard);
si->nr_discarding =
@@ -545,9 +549,9 @@ static int stat_show(struct seq_file *s, void *v)
si->nr_wb_cp_data, si->nr_wb_data,
si->nr_flushing, si->nr_flushed,
si->flush_list_empty);
- seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
- si->nr_discarding, si->nr_discarded,
- si->nr_discard_cmd, si->undiscard_blks);
+ seq_printf(s, "Discard: (%4d %4d, avg:%4lldns, peak:%4lldns))
cmd: %4d undiscard:%4u\n",
+ si->nr_discarding, si->nr_discarded,
ktime_to_us(si->discard_avg),
+ ktime_to_us(si->discard_peak), si->nr_discard_cmd,
si->undiscard_blks);
seq_printf(s, " - atomic IO: %4d (Max. %4d)\n",
si->aw_cnt, si->max_aw_cnt);
seq_printf(s, " - compress: %4d, hit:%8d\n",
si->compress_pages, si->compress_page_hit);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e8953c3dc81a..2cd55cb981ff 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -371,6 +371,8 @@ struct discard_cmd {
int error; /* bio error */
spinlock_t lock;/* for state/bio_ref updating */
unsigned short bio_ref; /* bio reference count */
+ struct discard_cmd_control *dcc;/* global discard cmd control */
+ ktime_t submit_start; /* submit start time */
};
enum {
@@ -415,6 +417,9 @@ struct discard_cmd_control {
unsigned int max_ordered_discard; /* maximum discard granularity
issued by lba order */
unsigned int undiscard_blks;/* # of undiscard blocks */
unsigned int next_pos; /* next discard position */
+ spinlock_t discard_time_lock; /* for discard time statistics */
+ ktime_t discard_time_avg; /* issued discard cmd avg time
*/
+ ktime_t discard_time_peak; /* issued discard cmd peak time
*/
atomic_t issued_discard;/* # of issued discard */
atomic_t queued_discard;/* # of queued discard */
atomic_t discard_cmd_cnt; /* # of cached cmd count */
@@ -3896,6 +3901,7 @@ struct f2fs_stat_info {
int nr_dio_read, nr_dio_write;
unsigned int io_skip_bggc, other_skip_bggc;
int nr_flushing, nr_flushed, flush_list_empty;
+ ktime_t discard_avg, discard_peak;
int nr_discarding, nr_discarded;
int nr_discard_cmd;
unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..73cd05bb3f4a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -937,6 +937,7 @@ static struct discard_cmd *__create_discard_cmd(struct
f2fs_sb_info *sbi,
list_add_tail(>list, pend_list);
spin_lock_init(>lock);
dc->bio_ref = 0;
+ dc->dcc = dcc;
atomic_inc(>discard_cmd_cnt);
dcc->undiscard_blks += len;
@@ -1006,9 +1007,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info
*sbi,
static void f2fs_submit_discard_endio(struct bio *bio)
{
struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+ struct discard_cmd_control *dcc = dc->dcc;
unsigned long flags;
+ ktime_t submit_time;
+ int nr_discarded;
spin_lock_irqsave(>lock, flags);
+ submit_time = ktime_sub(ktime_get(), dc->submit_start);
if (!dc->error)
dc->error = blk_status_to_errno(bio->bi_status);
dc->bio_ref--;
@@ -1018,6 +1023,16 @@ static void
Re: [f2fs-dev] [PATCH] f2fs: don't set FI_COMPRESS_RELEASED if file is not compressed
On 2022/12/10 4:19, Jaegeuk Kim wrote: On 12/08, zhoudan8 wrote: In compress_mode=user, f2fs_release_compress_blocks() does not verify whether it has been compressed and sets FI_COMPRESS_RELEASED directly. which will lead to return -EINVAL after calling compress. To fix it,let's do not set FI_COMPRESS_RELEASED if file is not compressed. Do you mean you want to avoid EINVAL on a file having FI_COMPRESS_RELEASED with zero i_compr_blokcs? I think the current logic is giving the error on a released file already. IMO, if i_compr_blocks is zero, it's better to return -EINVAL in f2fs_release_compress_blocks(), as there will be no benefit after the conversion. Thanks, Signed-off-by: zhoudan8 --- fs/f2fs/file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 82cda1258227..f32910077df6 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3451,14 +3451,13 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg) ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); if (ret) goto out; - - set_inode_flag(inode, FI_COMPRESS_RELEASED); inode->i_ctime = current_time(inode); f2fs_mark_inode_dirty_sync(inode, true); if (!atomic_read(_I(inode)->i_compr_blocks)) goto out; + set_inode_flag(inode, FI_COMPRESS_RELEASED); f2fs_down_write(_I(inode)->i_gc_rwsem[WRITE]); filemap_invalidate_lock(inode->i_mapping); -- 2.38.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: don't set FI_COMPRESS_RELEASED if file is not compressed
Maybe I'm not describing it clearly enough, but I think there is something wrong with the logic here.The 'f2fs_release_compress_blocks' method does not determine if the file is compressed, but simply adds the FI_COMPRESS_RELEASED flag. In particular, in the current Android system, when the application is installed, the release interface is called by default to release the storage marked as compressed, without checking whether the file is actually compressed. In this case, when compress_mode is set to user, calling the compress interface returns ENVAL and the file cannot be compressed. So I think the implementation of release needs to be modified, and only set FI_COMPRESS_RELEASED when it's really compressed and the storage is released. On Fri, Dec 09, 2022 at 12:19:44PM -0800, Jaegeuk Kim wrote: > On 12/08, zhoudan8 wrote: > > In compress_mode=user, f2fs_release_compress_blocks() > > does not verify whether it has been compressed and > > sets FI_COMPRESS_RELEASED directly. which will lead to > > return -EINVAL after calling compress. > > To fix it,let's do not set FI_COMPRESS_RELEASED if file > > is not compressed. > > Do you mean you want to avoid EINVAL on a file having FI_COMPRESS_RELEASED > with zero i_compr_blokcs? > > I think the current logic is giving the error on a released file already. > > > > > Signed-off-by: zhoudan8 > > --- > > fs/f2fs/file.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > > index 82cda1258227..f32910077df6 100644 > > --- a/fs/f2fs/file.c > > +++ b/fs/f2fs/file.c > > @@ -3451,14 +3451,13 @@ static int f2fs_release_compress_blocks(struct file > > *filp, unsigned long arg) > > ret = filemap_write_and_wait_range(inode->i_mapping, 0, LLONG_MAX); > > if (ret) > > goto out; > > - > > - set_inode_flag(inode, FI_COMPRESS_RELEASED); > > inode->i_ctime = current_time(inode); > > f2fs_mark_inode_dirty_sync(inode, true); > > > > if (!atomic_read(_I(inode)->i_compr_blocks)) > > goto out; > > > > + set_inode_flag(inode, FI_COMPRESS_RELEASED); > > f2fs_down_write(_I(inode)->i_gc_rwsem[WRITE]); > > filemap_invalidate_lock(inode->i_mapping); > > > > -- > > 2.38.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/2] f2fs: add support for counting the average time of submit discard cmd
On 2022/11/29 12:15, Yangtao Li wrote:
This patch adds support for counting the average time of submit discard
command, and we can see its value in debugfs.
How about enabling this only when CONFIG_DEBUG_FS is on?
+Cc block mailing list
Not sure block layer has similar stats? if it hasn't, can such stat
be accounted in block layer, and then all filesystem can be benefited.
Thanks,
Signed-off-by: Yangtao Li
---
fs/f2fs/debug.c | 7 +--
fs/f2fs/f2fs.h| 5 +
fs/f2fs/segment.c | 18 --
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 733b1bd37404..eed3edfc5faf 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -109,6 +109,9 @@ static void update_general_status(struct f2fs_sb_info *sbi)
llist_empty(_I(sbi)->fcc_info->issue_list);
}
if (SM_I(sbi)->dcc_info) {
+ struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
+
+ si->discard_avg = dcc->discard_time_avg;
si->nr_discarded =
atomic_read(_I(sbi)->dcc_info->issued_discard);
si->nr_discarding =
@@ -510,8 +513,8 @@ static int stat_show(struct seq_file *s, void *v)
si->nr_wb_cp_data, si->nr_wb_data,
si->nr_flushing, si->nr_flushed,
si->flush_list_empty);
- seq_printf(s, "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n",
- si->nr_discarding, si->nr_discarded,
+ seq_printf(s, "Discard: (%4d %4d, avg:%4lldns)) cmd: %4d
undiscard:%4u\n",
+ si->nr_discarding, si->nr_discarded,
ktime_to_us(si->discard_avg),
si->nr_discard_cmd, si->undiscard_blks);
seq_printf(s, " - atomic IO: %4d (Max. %4d)\n",
si->aw_cnt, si->max_aw_cnt);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eb8c27c4e5fc..5a99759d10ac 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -370,6 +370,8 @@ struct discard_cmd {
int error; /* bio error */
spinlock_t lock;/* for state/bio_ref updating */
unsigned short bio_ref; /* bio reference count */
+ struct discard_cmd_control *dcc; /* global discard cmd control */
+ ktime_t submit_start; /* submit start time */
};
enum {
@@ -414,6 +416,8 @@ struct discard_cmd_control {
unsigned int max_ordered_discard; /* maximum discard granularity
issued by lba order */
unsigned int undiscard_blks;/* # of undiscard blocks */
unsigned int next_pos; /* next discard position */
+ spinlock_t discard_time_lock; /* for discard time statistics */
+ ktime_t discard_time_avg; /* issued discard cmd avg time
*/
atomic_t issued_discard;/* # of issued discard */
atomic_t queued_discard;/* # of queued discard */
atomic_t discard_cmd_cnt; /* # of cached cmd count */
@@ -3882,6 +3886,7 @@ struct f2fs_stat_info {
int nr_dio_read, nr_dio_write;
unsigned int io_skip_bggc, other_skip_bggc;
int nr_flushing, nr_flushed, flush_list_empty;
+ ktime_t discard_avg;
int nr_discarding, nr_discarded;
int nr_discard_cmd;
unsigned int undiscard_blks;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9486ca49ecb1..bc96b1afb308 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -931,6 +931,7 @@ static struct discard_cmd *__create_discard_cmd(struct
f2fs_sb_info *sbi,
list_add_tail(>list, pend_list);
spin_lock_init(>lock);
dc->bio_ref = 0;
+ dc->dcc = dcc;
atomic_inc(>discard_cmd_cnt);
dcc->undiscard_blks += len;
@@ -1000,9 +1001,13 @@ static void __remove_discard_cmd(struct f2fs_sb_info *sbi,
static void f2fs_submit_discard_endio(struct bio *bio)
{
struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
+ struct discard_cmd_control *dcc = dc->dcc;
unsigned long flags;
+ ktime_t submit_time;
+ int nr_discarded;
spin_lock_irqsave(>lock, flags);
+ submit_time = ktime_sub(ktime_get(), dc->submit_start);
if (!dc->error)
dc->error = blk_status_to_errno(bio->bi_status);
dc->bio_ref--;
@@ -1012,6 +1017,14 @@ static void f2fs_submit_discard_endio(struct bio *bio)
}
spin_unlock_irqrestore(>lock, flags);
bio_put(bio);
+
+ spin_lock_irqsave(>discard_time_lock, flags);
+ nr_discarded = atomic_read(>issued_discard);
+ dcc->discard_time_avg = div_u64(ktime_add(nr_discarded *
dcc->discard_time_avg,
+
submit_time),
+
Re: [f2fs-dev] [PATCH v2] f2fs: reset wait_ms to default if any of the victims have been selected
On 2022/12/11 21:08, Yuwei Guan wrote: In non-foreground gc mode, if no victim is selected, the gc process will wait for no_gc_sleep_time before waking up again. In this subsequent time, even though a victim will be selected, the gc process still waits for no_gc_sleep_time before waking up. The configuration of wait_ms is not reasonable. After any of the victims have been selected, we need to reset wait_ms to default sleep time from no_gc_sleep_time. Signed-off-by: Yuwei Guan Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: add iostat support for FS_DISCARD_IO
On 2022/12/12 13:47, Yangtao Li wrote: Just like other data we count uses the number of bytes as the basic unit, but discard uses the number of cmds as the statistical unit. In fact, the discard command contains the number of blocks. In order to avoid breaking its usage of application, let's keeping FS_DISCARD as it is, and add FS_DISCARD_IO to account discard bytes. Suggested-by: Chao Yu Signed-off-by: Yangtao Li Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
