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 <jaeg...@kernel.org> > > 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 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. 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. > > 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 <jaeg...@kernel.org> > > --- > > 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 Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel