linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Chao Yu <yuchao0@huawei.com>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Jens Axboe <axboe@kernel.dk>
Cc: linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH] f2fs: remove request_list check in is_idle()
Date: Wed, 17 Oct 2018 09:18:21 +0800	[thread overview]
Message-ID: <2c67b341-15c3-a9d1-e52d-4b51e5be8920@huawei.com> (raw)
In-Reply-To: <20181016192314.GB29774@jaegeuk-macbookpro.roam.corp.google.com>

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 <jaegeuk@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.

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 <jaegeuk@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;
> 

  parent reply	other threads:[~2018-10-17  1:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16 14:34 [PATCH] f2fs: remove request_list check in is_idle() Jens Axboe
2018-10-16 16:06 ` Chao Yu
2018-10-16 16:20   ` Jens Axboe
2018-10-16 19:23     ` Jaegeuk Kim
2018-10-16 19:24       ` Jens Axboe
2018-10-16 19:31         ` Jaegeuk Kim
2018-10-16 19:36           ` Jens Axboe
2018-10-17  1:18       ` Chao Yu [this message]
2018-10-17  2:19         ` Jaegeuk Kim
2018-10-17  2:49           ` Chao Yu
2018-10-17  3:06             ` Jaegeuk Kim
2018-10-17  3:11       ` Chao Yu
2018-10-17  1:08     ` Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2c67b341-15c3-a9d1-e52d-4b51e5be8920@huawei.com \
    --to=yuchao0@huawei.com \
    --cc=axboe@kernel.dk \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).