LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: Daeho Jeong <daeho43@gmail.com>,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
	Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: introduce periodic iostat io latency traces
Date: Fri, 13 Aug 2021 11:56:06 -0700	[thread overview]
Message-ID: <YRbARsMfs2O2fz2s@google.com> (raw)
In-Reply-To: <b1a7b8a6-89dc-9076-2388-ced59aa8c47c@kernel.org>

On 08/13, Chao Yu wrote:
> On 2021/8/13 4:52, Jaegeuk Kim wrote:
> > On 08/11, Chao Yu wrote:
> > > Hi Daeho,
> > > 
> > > On 2021/8/4 6:55, Daeho Jeong wrote:
> > > > From: Daeho Jeong <daehojeong@google.com>
> > > > 
> > > > Whenever we notice some sluggish issues on our machines, we are always
> > > > curious about how well all types of I/O in the f2fs filesystem are
> > > > handled. But, it's hard to get this kind of real data. First of all,
> > > > we need to reproduce the issue while turning on the profiling tool like
> > > > blktrace, but the issue doesn't happen again easily. Second, with the
> > > > intervention of any tools, the overall timing of the issue will be
> > > > slightly changed and it sometimes makes us hard to figure it out.
> > > > 
> > > > So, I added F2FS_IOSTAT_IO_LATENCY config option to support printing out
> > > > IO latency statistics tracepoint events which are minimal things to
> > > > understand filesystem's I/O related behaviors. With "iostat_enable" sysfs
> > > > node on, we can get this statistics info in a periodic way and it
> > > > would cause the least overhead.
> > > > 
> > > > [samples]
> > > >    f2fs_ckpt-254:1-507     [003] ....  2842.439683: f2fs_iostat_latency:
> > > > dev = (254,11), iotype [peak lat.(ms)/avg lat.(ms)/count],
> > > > rd_data [136/1/801], rd_node [136/1/1704], rd_meta [4/2/4],
> > > > wr_sync_data [164/16/3331], wr_sync_node [152/3/648],
> > > > wr_sync_meta [160/2/4243], wr_async_data [24/13/15],
> > > > wr_async_node [0/0/0], wr_async_meta [0/0/0]
> > > > 
> > > >    f2fs_ckpt-254:1-507     [002] ....  2845.450514: f2fs_iostat_latency:
> > > > dev = (254,11), iotype [peak lat.(ms)/avg lat.(ms)/count],
> > > > rd_data [60/3/456], rd_node [60/3/1258], rd_meta [0/0/1],
> > > > wr_sync_data [120/12/2285], wr_sync_node [88/5/428],
> > > > wr_sync_meta [52/6/2990], wr_async_data [4/1/3],
> > > > wr_async_node [0/0/0], wr_async_meta [0/0/0]
> > > > 
> > > > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> > > > 
> > > > ---
> > > > v2: clean up with wrappers and fix a build breakage reported by
> > > >       kernel test robot <lkp@intel.com>
> > > > ---
> > > >    fs/f2fs/Kconfig             |   9 +++
> > > 
> > > I try to apply this patch in my local dev branch, but it failed due to
> > > conflicting with below commit, it needs to rebase this patch to last dev
> > > branch.
> > 
> > I applied this in dev branch. Could you please check?
> 
> Yeah, I see.
> 
> > > > +config F2FS_IOSTAT_IO_LATENCY
> > > > +	bool "F2FS IO statistics IO latency information"
> > > > +	depends on F2FS_FS
> > > > +	default n
> > > > +	help
> > > > +	  Support printing out periodic IO latency statistics tracepoint
> > > > +	  events. With this, you have to turn on "iostat_enable" sysfs
> > > > +	  node to print this out.
> > > 
> > > This functionality looks independent, how about introuducing iostat.h
> > > and iostat.c (not sure, maybe trace.[hc])to include newly added structure
> > > and functions for dispersive codes cleanup.
> 
> Thoughts? this also can avoid using CONFIG_F2FS_IOSTAT_IO_LATENCY in many places.

It seems there's somewhat dependency with iostat which is done by default.
How about adding this by default as well in the existing iostat, and then
covering all together by F2FS_IOSTAT?

> 
> Thanks,

  reply	other threads:[~2021-08-13 18:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 22:55 Daeho Jeong
2021-08-11  0:43 ` [f2fs-dev] " Chao Yu
2021-08-12 20:52   ` Jaegeuk Kim
2021-08-13  1:14     ` Chao Yu
2021-08-13 18:56       ` Jaegeuk Kim [this message]
2021-08-14  2:06         ` Chao Yu
2021-08-15  4:27           ` Daeho Jeong
2021-08-16  2:37             ` 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=YRbARsMfs2O2fz2s@google.com \
    --to=jaegeuk@kernel.org \
    --cc=chao@kernel.org \
    --cc=daeho43@gmail.com \
    --cc=daehojeong@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [f2fs-dev] [PATCH v2] f2fs: introduce periodic iostat io latency traces' \
    /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

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).