On 06/04, Chao Yu wrote:
> 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?
Okay, let's try that and see what'll happen.
Thanks,
>
> 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
