* [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
@ 2019-12-23 4:00 Chao Yu
2019-12-23 8:41 ` Geert Uytterhoeven
0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-12-23 4:00 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
As Geert Uytterhoeven reported:
for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
On some platforms, HZ can be less than 50, then unexpected 0 timeout
jiffies will be set in congestion_wait().
This patch introduces a macro DEFAULT_IO_TIMEOUT_JIFFIES to limit
mininum value of timeout jiffies.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/compress.c | 3 ++-
fs/f2fs/data.c | 5 +++--
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/gc.c | 3 ++-
fs/f2fs/inode.c | 3 ++-
fs/f2fs/node.c | 3 ++-
fs/f2fs/recovery.c | 6 ++++--
fs/f2fs/segment.c | 12 ++++++++----
fs/f2fs/super.c | 6 ++++--
9 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 1bc86a54ad71..ee4fe8e644aa 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -945,7 +945,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
} else if (ret == -EAGAIN) {
ret = 0;
cond_resched();
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
lock_page(cc->rpages[i]);
clear_page_dirty_for_io(cc->rpages[i]);
goto retry_write;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f1f5c701228d..78b5c0b0287e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2320,7 +2320,8 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
/* flush pending IOs and wait for a while in the ENOMEM case */
if (PTR_ERR(fio->encrypted_page) == -ENOMEM) {
f2fs_flush_merged_writes(fio->sbi);
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
gfp_flags |= __GFP_NOFAIL;
goto retry_encrypt;
}
@@ -2900,7 +2901,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
if (wbc->sync_mode == WB_SYNC_ALL) {
cond_resched();
congestion_wait(BLK_RW_ASYNC,
- HZ/50);
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry_write;
}
goto next;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 16edbf4e05e8..4bdc20a94185 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -559,6 +559,8 @@ enum {
#define DEFAULT_RETRY_IO_COUNT 8 /* maximum retry read IO count */
+#define DEFAULT_IO_TIMEOUT_JIFFIES (max_t(long, HZ/50, 1))
+
/* maximum retry quota flush count */
#define DEFAULT_RETRY_QUOTA_FLUSH_COUNT 8
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index b3d399623290..c9523c4e4001 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -970,7 +970,8 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
if (err) {
clear_cold_data(page);
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry;
}
if (is_dirty)
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 3fa728f40c2a..1646b4e7a79f 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -519,7 +519,8 @@ struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino)
inode = f2fs_iget(sb, ino);
if (IS_ERR(inode)) {
if (PTR_ERR(inode) == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry;
}
}
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 3314a0f3405e..94c2fa5811df 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2602,7 +2602,8 @@ int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
retry:
ipage = f2fs_grab_cache_page(NODE_MAPPING(sbi), ino, false);
if (!ipage) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry;
}
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 763d5c0951d1..72e0f30b7d99 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -535,7 +535,8 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
err = f2fs_get_dnode_of_data(&dn, start, ALLOC_NODE);
if (err) {
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry_dn;
}
goto out;
@@ -618,7 +619,8 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
err = check_index_in_prev_nodes(sbi, dest, &dn);
if (err) {
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry_prev;
}
goto err;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9519532c029..7cf2817bd83e 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -245,7 +245,8 @@ static int __revoke_inmem_pages(struct inode *inode,
LOOKUP_NODE);
if (err) {
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
cond_resched();
goto retry;
}
@@ -312,7 +313,8 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
skip:
iput(inode);
}
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
cond_resched();
if (gc_failure) {
if (++looped >= count)
@@ -415,7 +417,8 @@ static int __f2fs_commit_inmem_pages(struct inode *inode)
err = f2fs_do_write_data_page(&fio);
if (err) {
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
cond_resched();
goto retry;
}
@@ -2801,7 +2804,8 @@ static unsigned int __issue_discard_cmd_range(struct f2fs_sb_info *sbi,
blk_finish_plug(&plug);
mutex_unlock(&dcc->cmd_lock);
trimmed += __wait_all_discard_cmd(sbi, NULL);
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto next;
}
skip:
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 9d491f8fad4f..eff95e6d5641 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1891,7 +1891,8 @@ static ssize_t f2fs_quota_read(struct super_block *sb, int type, char *data,
page = read_cache_page_gfp(mapping, blkidx, GFP_NOFS);
if (IS_ERR(page)) {
if (PTR_ERR(page) == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto repeat;
}
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
@@ -1945,7 +1946,8 @@ static ssize_t f2fs_quota_write(struct super_block *sb, int type,
&page, NULL);
if (unlikely(err)) {
if (err == -ENOMEM) {
- congestion_wait(BLK_RW_ASYNC, HZ/50);
+ congestion_wait(BLK_RW_ASYNC,
+ DEFAULT_IO_TIMEOUT_JIFFIES);
goto retry;
}
set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-23 4:00 [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES Chao Yu
@ 2019-12-23 8:41 ` Geert Uytterhoeven
2019-12-25 9:58 ` Vyacheslav Dubeyko
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2019-12-23 8:41 UTC (permalink / raw)
To: Chao Yu
Cc: Jaegeuk Kim, linux-f2fs-devel, Linux Kernel Mailing List,
Chao Yu, Linux FS Devel
Hi,
CC linux-fsdevel
On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
> As Geert Uytterhoeven reported:
>
> for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
>
> On some platforms, HZ can be less than 50, then unexpected 0 timeout
> jiffies will be set in congestion_wait().
>
> This patch introduces a macro DEFAULT_IO_TIMEOUT_JIFFIES to limit
> mininum value of timeout jiffies.
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
Thanks for your patch!
> ---
> fs/f2fs/compress.c | 3 ++-
> fs/f2fs/data.c | 5 +++--
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/gc.c | 3 ++-
> fs/f2fs/inode.c | 3 ++-
> fs/f2fs/node.c | 3 ++-
> fs/f2fs/recovery.c | 6 ++++--
> fs/f2fs/segment.c | 12 ++++++++----
> fs/f2fs/super.c | 6 ++++--
> 9 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 1bc86a54ad71..ee4fe8e644aa 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -945,7 +945,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
> } else if (ret == -EAGAIN) {
> ret = 0;
> cond_resched();
> - congestion_wait(BLK_RW_ASYNC, HZ/50);
> + congestion_wait(BLK_RW_ASYNC,
> + DEFAULT_IO_TIMEOUT_JIFFIES);
> lock_page(cc->rpages[i]);
> clear_page_dirty_for_io(cc->rpages[i]);
> goto retry_write;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index f1f5c701228d..78b5c0b0287e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2320,7 +2320,8 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
> /* flush pending IOs and wait for a while in the ENOMEM case */
> if (PTR_ERR(fio->encrypted_page) == -ENOMEM) {
> f2fs_flush_merged_writes(fio->sbi);
> - congestion_wait(BLK_RW_ASYNC, HZ/50);
> + congestion_wait(BLK_RW_ASYNC,
> + DEFAULT_IO_TIMEOUT_JIFFIES);
> gfp_flags |= __GFP_NOFAIL;
> goto retry_encrypt;
> }
> @@ -2900,7 +2901,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> if (wbc->sync_mode == WB_SYNC_ALL) {
> cond_resched();
> congestion_wait(BLK_RW_ASYNC,
> - HZ/50);
> + DEFAULT_IO_TIMEOUT_JIFFIES);
> goto retry_write;
> }
> goto next;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 16edbf4e05e8..4bdc20a94185 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -559,6 +559,8 @@ enum {
>
> #define DEFAULT_RETRY_IO_COUNT 8 /* maximum retry read IO count */
>
> +#define DEFAULT_IO_TIMEOUT_JIFFIES (max_t(long, HZ/50, 1))
> +
> /* maximum retry quota flush count */
> #define DEFAULT_RETRY_QUOTA_FLUSH_COUNT 8
>
Seeing other file systems (ext4, xfs) and even core MM code suffers from
the same issue, perhaps it makes sense to move this into congestion_wait(),
i.e. increase the timeout to 1 if it's zero in the latter function?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-23 8:41 ` Geert Uytterhoeven
@ 2019-12-25 9:58 ` Vyacheslav Dubeyko
2019-12-26 9:38 ` Chao Yu
2019-12-26 10:43 ` Geert Uytterhoeven
2019-12-26 3:42 ` Chao Yu
2019-12-31 12:52 ` Matthew Wilcox
2 siblings, 2 replies; 9+ messages in thread
From: Vyacheslav Dubeyko @ 2019-12-25 9:58 UTC (permalink / raw)
To: Geert Uytterhoeven, Chao Yu
Cc: Jaegeuk Kim, linux-f2fs-devel, Linux Kernel Mailing List,
Chao Yu, Linux FS Devel
On Mon, 2019-12-23 at 09:41 +0100, Geert Uytterhoeven wrote:
> Hi,
>
> CC linux-fsdevel
>
> On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
> > As Geert Uytterhoeven reported:
> >
> > for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
> >
> > On some platforms, HZ can be less than 50, then unexpected 0
> > timeout
> > jiffies will be set in congestion_wait().
> >
It looks like that HZ could have various value on diferent platforms.
So, why does it need to divide HZ on 50? Does it really necessary?
Could it be used HZ only without the division operation?
Thanks,
Viacheslav Dubeyko.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-23 8:41 ` Geert Uytterhoeven
2019-12-25 9:58 ` Vyacheslav Dubeyko
@ 2019-12-26 3:42 ` Chao Yu
2019-12-31 12:52 ` Matthew Wilcox
2 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-12-26 3:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jaegeuk Kim, linux-f2fs-devel, Linux Kernel Mailing List,
Chao Yu, Linux FS Devel
On 2019/12/23 16:41, Geert Uytterhoeven wrote:
> Hi,
>
> CC linux-fsdevel
>
> On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
>> As Geert Uytterhoeven reported:
>>
>> for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
>>
>> On some platforms, HZ can be less than 50, then unexpected 0 timeout
>> jiffies will be set in congestion_wait().
>>
>> This patch introduces a macro DEFAULT_IO_TIMEOUT_JIFFIES to limit
>> mininum value of timeout jiffies.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>
> Thanks for your patch!
>
>> ---
>> fs/f2fs/compress.c | 3 ++-
>> fs/f2fs/data.c | 5 +++--
>> fs/f2fs/f2fs.h | 2 ++
>> fs/f2fs/gc.c | 3 ++-
>> fs/f2fs/inode.c | 3 ++-
>> fs/f2fs/node.c | 3 ++-
>> fs/f2fs/recovery.c | 6 ++++--
>> fs/f2fs/segment.c | 12 ++++++++----
>> fs/f2fs/super.c | 6 ++++--
>> 9 files changed, 29 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 1bc86a54ad71..ee4fe8e644aa 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -945,7 +945,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
>> } else if (ret == -EAGAIN) {
>> ret = 0;
>> cond_resched();
>> - congestion_wait(BLK_RW_ASYNC, HZ/50);
>> + congestion_wait(BLK_RW_ASYNC,
>> + DEFAULT_IO_TIMEOUT_JIFFIES);
>> lock_page(cc->rpages[i]);
>> clear_page_dirty_for_io(cc->rpages[i]);
>> goto retry_write;
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index f1f5c701228d..78b5c0b0287e 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2320,7 +2320,8 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
>> /* flush pending IOs and wait for a while in the ENOMEM case */
>> if (PTR_ERR(fio->encrypted_page) == -ENOMEM) {
>> f2fs_flush_merged_writes(fio->sbi);
>> - congestion_wait(BLK_RW_ASYNC, HZ/50);
>> + congestion_wait(BLK_RW_ASYNC,
>> + DEFAULT_IO_TIMEOUT_JIFFIES);
>> gfp_flags |= __GFP_NOFAIL;
>> goto retry_encrypt;
>> }
>> @@ -2900,7 +2901,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>> if (wbc->sync_mode == WB_SYNC_ALL) {
>> cond_resched();
>> congestion_wait(BLK_RW_ASYNC,
>> - HZ/50);
>> + DEFAULT_IO_TIMEOUT_JIFFIES);
>> goto retry_write;
>> }
>> goto next;
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 16edbf4e05e8..4bdc20a94185 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -559,6 +559,8 @@ enum {
>>
>> #define DEFAULT_RETRY_IO_COUNT 8 /* maximum retry read IO count */
>>
>> +#define DEFAULT_IO_TIMEOUT_JIFFIES (max_t(long, HZ/50, 1))
>> +
>> /* maximum retry quota flush count */
>> #define DEFAULT_RETRY_QUOTA_FLUSH_COUNT 8
>>
>
> Seeing other file systems (ext4, xfs) and even core MM code suffers from
> the same issue, perhaps it makes sense to move this into congestion_wait(),
> i.e. increase the timeout to 1 if it's zero in the latter function?
Yup, maybe I can submit a RFC patch to change congestion_wait(), before that
we still need this f2fs change.
Thanks,
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-25 9:58 ` Vyacheslav Dubeyko
@ 2019-12-26 9:38 ` Chao Yu
2019-12-26 10:43 ` Geert Uytterhoeven
1 sibling, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-12-26 9:38 UTC (permalink / raw)
To: Vyacheslav Dubeyko, Geert Uytterhoeven
Cc: Jaegeuk Kim, linux-f2fs-devel, Linux Kernel Mailing List,
Chao Yu, Linux FS Devel
On 2019/12/25 17:58, Vyacheslav Dubeyko wrote:
> On Mon, 2019-12-23 at 09:41 +0100, Geert Uytterhoeven wrote:
>> Hi,
>>
>> CC linux-fsdevel
>>
>> On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
>>> As Geert Uytterhoeven reported:
>>>
>>> for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
>>>
>>> On some platforms, HZ can be less than 50, then unexpected 0
>>> timeout
>>> jiffies will be set in congestion_wait().
>>>
>
>
> It looks like that HZ could have various value on diferent platforms.
> So, why does it need to divide HZ on 50? Does it really necessary?
I guess this code was copied from other filesystems, I have no idea why
we should use HZ/50 as timeout interval value.
> Could it be used HZ only without the division operation?
Actually, as Geert pointed out, we can handle that zeroed value parameter
inside congestion_wait() to cover all filesystems use cases.
Thanks,
>
> Thanks,
> Viacheslav Dubeyko.
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-25 9:58 ` Vyacheslav Dubeyko
2019-12-26 9:38 ` Chao Yu
@ 2019-12-26 10:43 ` Geert Uytterhoeven
2019-12-26 13:08 ` Vyacheslav Dubeyko
1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2019-12-26 10:43 UTC (permalink / raw)
To: Vyacheslav Dubeyko
Cc: Chao Yu, Jaegeuk Kim, linux-f2fs-devel,
Linux Kernel Mailing List, Chao Yu, Linux FS Devel
Hi Vyacheslav,
On Wed, Dec 25, 2019 at 10:58 AM Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Mon, 2019-12-23 at 09:41 +0100, Geert Uytterhoeven wrote:
> > On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
> > > As Geert Uytterhoeven reported:
> > >
> > > for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
> > >
> > > On some platforms, HZ can be less than 50, then unexpected 0
> > > timeout
> > > jiffies will be set in congestion_wait().
>
> It looks like that HZ could have various value on diferent platforms.
> So, why does it need to divide HZ on 50? Does it really necessary?
> Could it be used HZ only without the division operation?
A timeout of HZ means 1 second.
HZ/50 means 20 ms, but has the risk of being zero, if HZ < 50.
If you want to use a timeout of 20 ms, you best use msecs_to_jiffies(20),
as that takes care of the special cases, and never returns 0.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-26 10:43 ` Geert Uytterhoeven
@ 2019-12-26 13:08 ` Vyacheslav Dubeyko
2019-12-26 13:16 ` Geert Uytterhoeven
0 siblings, 1 reply; 9+ messages in thread
From: Vyacheslav Dubeyko @ 2019-12-26 13:08 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Chao Yu, Jaegeuk Kim, linux-f2fs-devel,
Linux Kernel Mailing List, Chao Yu, Linux FS Devel
Hi Geert,
On Thu, 2019-12-26 at 11:43 +0100, Geert Uytterhoeven wrote:
> Hi Vyacheslav,
>
> On Wed, Dec 25, 2019 at 10:58 AM Vyacheslav Dubeyko <
> slava@dubeyko.com> wrote:
> > On Mon, 2019-12-23 at 09:41 +0100, Geert Uytterhoeven wrote:
> > > On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com>
> > > wrote:
> > > > As Geert Uytterhoeven reported:
> > > >
> > > > for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
> > > >
> > > > On some platforms, HZ can be less than 50, then unexpected 0
> > > > timeout
> > > > jiffies will be set in congestion_wait().
> >
> > It looks like that HZ could have various value on diferent
> > platforms.
> > So, why does it need to divide HZ on 50? Does it really necessary?
> > Could it be used HZ only without the division operation?
>
> A timeout of HZ means 1 second.
> HZ/50 means 20 ms, but has the risk of being zero, if HZ < 50.
>
> If you want to use a timeout of 20 ms, you best use
> msecs_to_jiffies(20),
> as that takes care of the special cases, and never returns 0.
>
The msecs_to_jiffies(20) looks much better for my taste. Maybe, we
could use this as solution of the issue?
Thanks,
Viacheslav Dubeyko.
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-26 13:08 ` Vyacheslav Dubeyko
@ 2019-12-26 13:16 ` Geert Uytterhoeven
0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2019-12-26 13:16 UTC (permalink / raw)
To: Vyacheslav Dubeyko
Cc: Chao Yu, Jaegeuk Kim, linux-f2fs-devel,
Linux Kernel Mailing List, Chao Yu, Linux FS Devel
Hi Vyacheslav,
On Thu, Dec 26, 2019 at 2:08 PM Vyacheslav Dubeyko <slava@dubeyko.com> wrote:
> On Thu, 2019-12-26 at 11:43 +0100, Geert Uytterhoeven wrote:
> > On Wed, Dec 25, 2019 at 10:58 AM Vyacheslav Dubeyko <
> > slava@dubeyko.com> wrote:
> > > On Mon, 2019-12-23 at 09:41 +0100, Geert Uytterhoeven wrote:
> > > > On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com>
> > > > wrote:
> > > > > As Geert Uytterhoeven reported:
> > > > >
> > > > > for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
> > > > >
> > > > > On some platforms, HZ can be less than 50, then unexpected 0
> > > > > timeout
> > > > > jiffies will be set in congestion_wait().
> > >
> > > It looks like that HZ could have various value on diferent
> > > platforms.
> > > So, why does it need to divide HZ on 50? Does it really necessary?
> > > Could it be used HZ only without the division operation?
> >
> > A timeout of HZ means 1 second.
> > HZ/50 means 20 ms, but has the risk of being zero, if HZ < 50.
> >
> > If you want to use a timeout of 20 ms, you best use
> > msecs_to_jiffies(20),
> > as that takes care of the special cases, and never returns 0.
> >
>
> The msecs_to_jiffies(20) looks much better for my taste. Maybe, we
> could use this as solution of the issue?
Thanks, sounds good to me.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES
2019-12-23 8:41 ` Geert Uytterhoeven
2019-12-25 9:58 ` Vyacheslav Dubeyko
2019-12-26 3:42 ` Chao Yu
@ 2019-12-31 12:52 ` Matthew Wilcox
2 siblings, 0 replies; 9+ messages in thread
From: Matthew Wilcox @ 2019-12-31 12:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Chao Yu, Jaegeuk Kim, linux-f2fs-devel,
Linux Kernel Mailing List, Chao Yu, Linux FS Devel
On Mon, Dec 23, 2019 at 09:41:02AM +0100, Geert Uytterhoeven wrote:
> Hi,
>
> CC linux-fsdevel
>
> On Mon, Dec 23, 2019 at 5:01 AM Chao Yu <yuchao0@huawei.com> wrote:
> > As Geert Uytterhoeven reported:
> >
> > for parameter HZ/50 in congestion_wait(BLK_RW_ASYNC, HZ/50);
> >
> > On some platforms, HZ can be less than 50, then unexpected 0 timeout
> > jiffies will be set in congestion_wait().
It doesn't matter, congestion is broken and has been for years.
https://lore.kernel.org/linux-mm/20190923111900.GH15392@bombadil.infradead.org/
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-12-31 12:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-23 4:00 [PATCH] f2fs: introduce DEFAULT_IO_TIMEOUT_JIFFIES Chao Yu
2019-12-23 8:41 ` Geert Uytterhoeven
2019-12-25 9:58 ` Vyacheslav Dubeyko
2019-12-26 9:38 ` Chao Yu
2019-12-26 10:43 ` Geert Uytterhoeven
2019-12-26 13:08 ` Vyacheslav Dubeyko
2019-12-26 13:16 ` Geert Uytterhoeven
2019-12-26 3:42 ` Chao Yu
2019-12-31 12:52 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).