On 2018/6/2 2:35, Jaegeuk Kim wrote:
> On 06/01, heyunlei wrote:
>>
>>
>>> -----Original Message-----
>>> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
>>> Sent: Friday, June 01, 2018 1:56 AM
>>> To: heyunlei
>>> Cc: Yuchao (T); linux-f2fs-devel@lists.sourceforge.net; Wangbintian;
>>> Zhangdianfang (Euler)
>>> Subject: Re: [f2fs-dev][PATCH RFC] Revert "f2fs: avoid cpu lockup"
>>>
>>> Hi Yunlei,
>>>
>> Hi Jaegeuk，
>>> On 05/31, Yunlei He wrote:
>>>> This reverts commit 4db08d016ccedb5b97869724a096990acad59685 to
>>>> fix hungtask problem which can be reproduced as follow:
>>>>
>>>> Thread 0~3:
>>>>
>>>> while true
>>>> do
>>>> touch /xxx/test/file_xxx
>>>> done
>>>>
>>>> Thread 5 write a new checkpoint every three seconds.
>>>>
>>>> In the meantime, fio start 16 threads for randwrite.
>>>>
>>>> With my debug info, cycles num will exceed 1000 in function
>>>> f2fs_sync_dirty_inodes, and most of cycle will be dropped
>>>> into congestion_wait() and sleep more than 20ms.
>>>
>>> Original patch tries to fix watchdog which sync_dirty_inodes() grabs one
>>> cpu for a long time. Have you tried reproduce that as well?
>>>
>>
>> I have test this patch with increasing touch file threads to 32，and other
>> remain
>> same as previous, But I have not reproduce your problem.
>>
>> Besides, I try to test the code just remove this line:
>> congestion_wait(BLK_RW_ASYNC, HZ/50);
>>
>> It's also ok to my test, and cycle num reduced to < 3
>
> You may be able to add threshold here?
Hmm, if cycle exceeds threshold, still we can encounter this issue. How about
just removing congestion_wait, and keep cond_resched, it can switch current cpu
out?
Thanks,
>
>>
>> Thanks.
>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Yunlei He <heyun...@huawei.com>
>>>> ---
>>>> fs/f2fs/checkpoint.c | 10 ----------
>>>> 1 file changed, 10 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index 8eb184c..76e1856 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -944,7 +944,6 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi,
>>>> enum inode_type type)
>>>> struct inode *inode;
>>>> struct f2fs_inode_info *fi;
>>>> bool is_dir = (type == DIR_INODE);
>>>> - unsigned long ino = 0;
>>>>
>>>> trace_f2fs_sync_dirty_inodes_enter(sbi->sb, is_dir,
>>>> get_pages(sbi, is_dir ?
>>>> @@ -967,8 +966,6 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi,
>>>> enum inode_type type)
>>>> inode = igrab(&fi->vfs_inode);
>>>> spin_unlock(&sbi->inode_lock[type]);
>>>> if (inode) {
>>>> - unsigned long cur_ino = inode->i_ino;
>>>> -
>>>> if (is_dir)
>>>> F2FS_I(inode)->cp_task = current;
>>>>
>>>> @@ -978,13 +975,6 @@ int f2fs_sync_dirty_inodes(struct f2fs_sb_info *sbi,
>>>> enum inode_type type)
>>>> F2FS_I(inode)->cp_task = NULL;
>>>>
>>>> iput(inode);
>>>> - /* We need to give cpu to another writers. */
>>>> - if (ino == cur_ino) {
>>>> - congestion_wait(BLK_RW_ASYNC, HZ/50);
>>>> - cond_resched();
>>>> - } else {
>>>> - ino = cur_ino;
>>>> - }
>>>> } else {
>>>> /*
>>>> * We should submit bio, since it exists several
>>>> --
>>>> 1.9.1
>
> .
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
