On 2018/10/17 10:19, Jaegeuk Kim wrote: > On 10/17, Chao Yu wrote: >> Hi Jaegeuk, Jens, >> >> On 2018/10/17 3:23, Jaegeuk Kim wrote: >>> Thanks Jens, >>> >>> On top of the patch killing the dead code, I wrote another one to detect >>> the idle time by the internal account logic like below. IMHO, it'd be >>> better to decouple f2fs with other layers, if possible. >>> >>> Thanks, >>> >>> >From 85daf5190671b3d98ef779bdea77b4a046658708 Mon Sep 17 00:00:00 2001 >>> From: Jaegeuk Kim <[email protected]> >>> Date: Tue, 16 Oct 2018 10:20:53 -0700 >>> Subject: [PATCH] f2fs: account read IOs and use IO counts for is_idle >>> >>> This patch adds issued read IO counts which is under block layer. >> >> I think the problem here is, in android environment, there is multiple >> kinds of filesystems used in one device (emmc or ufs), so, if f2fs only be >> aware of IOs from itself, still we can face IO conflict between f2fs and >> other filesystem (ext4 used in system/... partition), it is possible when >> we are trigger GC/discard intensively such as in gc_urgent mode, user can >> be stuck when loading data from system partition. > > IMO, we don't need to worry about gc_urgent too much, since it will be set at
For most of the time, it's okay, in android idle time (IIRC, in middle night, charging, screen shutdowns for a while) can be broken by user who has bad habit that using cell phone in middle night randomly.... then user operation can be stuck due to IO busy storage. > the given android idle time. Other than that, I think it won't trigger GC and > discard too frequently which would be able to affect /system operations. How about considering low-end products? since performance of storage device is real bad, handling discard/read IO in /data can affect /system operations. > The new interface would be nice to have, but give more complexity on > backporting > work in old kernels and newer as well across the layers. Yes, that's a problem. BTW, we just encounter 'GC not work' problem in product, I guess that's due to is_idle()'s problem, let's try to confirm the root cause. Thanks, > >> >> So I guess the better way to avoid this is to export IO busy status in >> block layer to filesystem, in f2fs, we can provider effective service in >> background thread at real idle time, and will not impact user. >> >> Any thoughts? >> >> Thanks, >> >>> >>> Signed-off-by: Jaegeuk Kim <[email protected]> >>> --- >>> fs/f2fs/data.c | 24 ++++++++++++++++++++++-- >>> fs/f2fs/debug.c | 7 ++++++- >>> fs/f2fs/f2fs.h | 22 ++++++++++++++++------ >>> 3 files changed, 44 insertions(+), 9 deletions(-) >>> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>> index 8952f2d610a6..5fdc8d751f19 100644 >>> --- a/fs/f2fs/data.c >>> +++ b/fs/f2fs/data.c >>> @@ -52,6 +52,23 @@ static bool __is_cp_guaranteed(struct page *page) >>> return false; >>> } >>> >>> +static enum count_type __read_io_type(struct page *page) >>> +{ >>> + struct address_space *mapping = page->mapping; >>> + >>> + if (mapping) { >>> + struct inode *inode = mapping->host; >>> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode); >>> + >>> + if (inode->i_ino == F2FS_META_INO(sbi)) >>> + return F2FS_RD_META; >>> + >>> + if (inode->i_ino == F2FS_NODE_INO(sbi)) >>> + return F2FS_RD_NODE; >>> + } >>> + return F2FS_RD_DATA; >>> +} >>> + >>> /* postprocessing steps for read bios */ >>> enum bio_post_read_step { >>> STEP_INITIAL = 0, >>> @@ -82,6 +99,7 @@ static void __read_end_io(struct bio *bio) >>> } else { >>> SetPageUptodate(page); >>> } >>> + dec_page_count(F2FS_P_SB(page), __read_io_type(page)); >>> unlock_page(page); >>> } >>> if (bio->bi_private) >>> @@ -464,8 +482,8 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio) >>> >>> __submit_bio(fio->sbi, bio, fio->type); >>> >>> - if (!is_read_io(fio->op)) >>> - inc_page_count(fio->sbi, WB_DATA_TYPE(fio->page)); >>> + inc_page_count(fio->sbi, is_read_io(fio->op) ? >>> + __read_io_type(page): WB_DATA_TYPE(fio->page)); >>> return 0; >>> } >>> >>> @@ -595,6 +613,7 @@ static int f2fs_submit_page_read(struct inode *inode, >>> struct page *page, >>> } >>> ClearPageError(page); >>> __submit_bio(F2FS_I_SB(inode), bio, DATA); >>> + inc_page_count(F2FS_I_SB(inode), F2FS_RD_DATA); >>> return 0; >>> } >>> >>> @@ -1593,6 +1612,7 @@ static int f2fs_mpage_readpages(struct address_space >>> *mapping, >>> if (bio_add_page(bio, page, blocksize, 0) < blocksize) >>> goto submit_and_realloc; >>> >>> + inc_page_count(F2FS_I_SB(inode), F2FS_RD_DATA); >>> ClearPageError(page); >>> last_block_in_bio = block_nr; >>> goto next_page; >>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c >>> index 026e10f30889..139b4d5c83d5 100644 >>> --- a/fs/f2fs/debug.c >>> +++ b/fs/f2fs/debug.c >>> @@ -55,6 +55,9 @@ static void update_general_status(struct f2fs_sb_info >>> *sbi) >>> si->max_vw_cnt = atomic_read(&sbi->max_vw_cnt); >>> si->nr_wb_cp_data = get_pages(sbi, F2FS_WB_CP_DATA); >>> si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA); >>> + si->nr_rd_data = get_pages(sbi, F2FS_RD_DATA); >>> + si->nr_rd_node = get_pages(sbi, F2FS_RD_NODE); >>> + si->nr_rd_meta = get_pages(sbi, F2FS_RD_META); >>> if (SM_I(sbi) && SM_I(sbi)->fcc_info) { >>> si->nr_flushed = >>> atomic_read(&SM_I(sbi)->fcc_info->issued_flush); >>> @@ -371,7 +374,9 @@ static int stat_show(struct seq_file *s, void *v) >>> seq_printf(s, " - Inner Struct Count: tree: %d(%d), node: >>> %d\n", >>> si->ext_tree, si->zombie_tree, si->ext_node); >>> seq_puts(s, "\nBalancing F2FS Async:\n"); >>> - seq_printf(s, " - IO (CP: %4d, Data: %4d, Flush: (%4d %4d >>> %4d), " >>> + seq_printf(s, " - IO_R (Data: %4d, Node: %4d, Meta: %4d\n", >>> + si->nr_rd_data, si->nr_rd_node, si->nr_rd_meta); >>> + seq_printf(s, " - IO_W (CP: %4d, Data: %4d, Flush: (%4d %4d >>> %4d), " >>> "Discard: (%4d %4d)) cmd: %4d undiscard:%4u\n", >>> si->nr_wb_cp_data, si->nr_wb_data, >>> si->nr_flushing, si->nr_flushed, >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 156e6cd2c812..5c80eca194b5 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -950,6 +950,9 @@ enum count_type { >>> F2FS_DIRTY_IMETA, >>> F2FS_WB_CP_DATA, >>> F2FS_WB_DATA, >>> + F2FS_RD_DATA, >>> + F2FS_RD_NODE, >>> + F2FS_RD_META, >>> NR_COUNT_TYPE, >>> }; >>> >>> @@ -1392,11 +1395,6 @@ static inline unsigned int f2fs_time_to_wait(struct >>> f2fs_sb_info *sbi, >>> return wait_ms; >>> } >>> >>> -static inline bool is_idle(struct f2fs_sb_info *sbi, int type) >>> -{ >>> - return f2fs_time_over(sbi, type); >>> -} >>> - >>> /* >>> * Inline functions >>> */ >>> @@ -1787,7 +1785,9 @@ static inline void inc_page_count(struct f2fs_sb_info >>> *sbi, int count_type) >>> atomic_inc(&sbi->nr_pages[count_type]); >>> >>> if (count_type == F2FS_DIRTY_DATA || count_type == F2FS_INMEM_PAGES || >>> - count_type == F2FS_WB_CP_DATA || count_type == F2FS_WB_DATA) >>> + count_type == F2FS_WB_CP_DATA || count_type == F2FS_WB_DATA || >>> + count_type == F2FS_RD_DATA || count_type == F2FS_RD_NODE || >>> + count_type == F2FS_RD_META) >>> return; >>> >>> set_sbi_flag(sbi, SBI_IS_DIRTY); >>> @@ -2124,6 +2124,15 @@ static inline struct bio *f2fs_bio_alloc(struct >>> f2fs_sb_info *sbi, >>> return bio_alloc(GFP_KERNEL, npages); >>> } >>> >>> +static inline bool is_idle(struct f2fs_sb_info *sbi, int type) >>> +{ >>> + if (get_pages(sbi, F2FS_RD_DATA) || get_pages(sbi, F2FS_RD_NODE) || >>> + get_pages(sbi, F2FS_RD_META) || get_pages(sbi, F2FS_WB_DATA) || >>> + get_pages(sbi, F2FS_WB_CP_DATA)) >>> + return false; >>> + return f2fs_time_over(sbi, type); >>> +} >>> + >>> static inline void f2fs_radix_tree_insert(struct radix_tree_root *root, >>> unsigned long index, void *item) >>> { >>> @@ -3116,6 +3125,7 @@ struct f2fs_stat_info { >>> int free_nids, avail_nids, alloc_nids; >>> int total_count, utilization; >>> int bg_gc, nr_wb_cp_data, nr_wb_data; >>> + int nr_rd_data, nr_rd_node, nr_rd_meta; >>> unsigned int io_skip_bggc, other_skip_bggc; >>> int nr_flushing, nr_flushed, flush_list_empty; >>> int nr_discarding, nr_discarded; >>> > > . > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
