Messages in this thread | | | Subject | Re: [PATCH v4] f2fs: add bio cache for IPU | From | Chao Yu <> | Date | Tue, 3 Sep 2019 07:34:53 +0800 |
| |
On 2019-9-3 7:04, Jaegeuk Kim wrote: > On 09/02, Chao Yu wrote: >> On 2019/9/1 15:17, Jaegeuk Kim wrote: >>> On 08/31, Chao Yu wrote: >>>> On 2019/2/19 16:15, Chao Yu wrote: >>>>> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted, >>>>> } >>>>> >>>>> unlock_page(page); >>>>> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) >>>>> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) { >>>>> + f2fs_submit_ipu_bio(sbi, bio, page); >>>>> f2fs_balance_fs(sbi, need_balance_fs); >>>>> + } >>>> >>>> Above bio submission was added to avoid below deadlock: >>>> >>>> - __write_data_page >>>> - f2fs_do_write_data_page >>>> - set_page_writeback ---- set writeback flag >>>> - f2fs_inplace_write_data >>>> - f2fs_balance_fs >>>> - f2fs_gc >>>> - do_garbage_collect >>>> - gc_data_segment >>>> - move_data_page >>>> - f2fs_wait_on_page_writeback >>>> - wait_on_page_writeback --- wait writeback >>>> >>>> However, it breaks the merge of IPU IOs, to solve this issue, it looks we need >>>> to add global bio cache for such IPU merge case, then later >>>> f2fs_wait_on_page_writeback can check whether writebacked page is cached or not, >>>> and do the submission if necessary. >>>> >>>> Jaegeuk, any thoughts? >>> >>> How about calling f2fs_submit_ipu_bio() when we need to do GC in the same >>> context? >> >> However it also could happen in race case: >> >> Thread A Thread B >> - __write_data_page (inode x, page y) >> - f2fs_do_write_data_page >> - set_page_writeback ---- set writeback flag in page y >> - f2fs_inplace_write_data >> - f2fs_balance_fs >> - lock gc_mutex >> - lock gc_mutex >> - f2fs_gc >> - do_garbage_collect >> - gc_data_segment >> - move_data_page >> - f2fs_wait_on_page_writeback >> - wait_on_page_writeback --- wait writeback of page y >> >> So it needs a global bio cache for merged IPU pages, how about adding a list to >> link all ipu bios in struct f2fs_bio_info? > > Hmm, I can't think of better solution than adding a list. In this case, blk_plug > doesn't work well?
Only submitted bio will be taken care of plug, for our case, the bio is still pending though, I guess plug won't help.
Thanks,
> >> >> struct f2fs_bio_info { >> .... >> struct list_head ipu_bio_list; /* track all ipu bio */ >> spinlock_t ipu_bio_lock; /* protect ipu bio list */ >> } >> >>> >>>> >>>> Thanks, >>> . >>>
| |