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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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
>
