public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org,
	kernel-team@fb.com, avanzini.arianna@gmail.com
Subject: Re: [PATCH 09/10] blkcg: move io_service_bytes and io_serviced stats into blkcg_gq
Date: Wed, 15 Jul 2015 12:29:52 -0400	[thread overview]
Message-ID: <20150715162952.GA28818@redhat.com> (raw)
In-Reply-To: <20150715160425.GE15934@mtj.duckdns.org>

On Wed, Jul 15, 2015 at 12:04:25PM -0400, Tejun Heo wrote:
> Hello, Vivek.
> 
> On Tue, Jul 14, 2015 at 12:09:08PM -0400, Vivek Goyal wrote:
> > So now blkio.io_serviced will switch to accounting number of bios
> > instead of number of requests? I feel given other stats, things
> > are still confusing as other stats will similar name give stats
> > about requests and not bios.
> > 
> > IMHO, for a policy, either all the stats should be in bio or in terms
> > of requests. Having a mix of these is even more confusing.
> 
> Well, the actual problem is that we have so many stats which are
> hardly useful except for debugging blkcg itself.  Most of these stats
> aren't meaningful to userland.
> 
> > For example, IIUC, now blkio.io_serviced will keep count in terms of
> > bios while blkio.io_queued will keep count in terms of number of
> > requests.
> 
> Why does that matter tho?  io_queued tracks the number of requests
> currently queued.  It's not an accumulative stat.  It isn't possible
> to meaningfully correlate that stat with anything else there.
> 
> > If we are keeping common stats at block layer (instead of per policy),
> > I am wondering if it will make sense to reflect that in new cgroup
> > files which are common to all policies in that cgroup, instead of being per
> > policy. And deperecate respective per policy stat files over a period of time.
> 
> So, that's the plan for unified hierarchy and this is the groundwork
> for that.  There's no point in disturbing interface for the
> traditional hierarchies at this point.  We can simply add the new
> stats and use the new ones only on the unified hierarchy but frankly
> how many identical stats should we keep?  What we're doing is already
> pretty silly and I don't really want to add more on top.

Hi Tejun,

Ok. I am personally little apprehensive of changing the meaning of 
current stat, but I can live with it.

Can you please also update the blkio-controller.txt to reflect these
changes. I think following sections would require updation.

blkio.throttle.io_serviced
blkio.throttle.io_service_bytes

And we could mention in blkio.io_serviced that accounting is terms of
numeber of bios.

Thanks
Vivek

  reply	other threads:[~2015-07-15 16:29 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-12 18:00 [PATCHSET v2 block/for-4.3] blkcg: blkcg stats cleanup Tejun Heo
2015-07-12 18:00 ` [PATCH 01/10] cgroup: make cftype->private a unsigned long Tejun Heo
2015-08-11 17:36   ` Tejun Heo
2015-07-12 18:00 ` [PATCH 02/10] blkcg: inline [__]blkg_lookup() Tejun Heo
2015-07-12 18:00 ` [PATCH 03/10] blkcg: move root blkg lookup optimization from throtl_lookup_tg() to __blkg_lookup() Tejun Heo
2015-07-12 18:00 ` [PATCH 04/10] blk-throttle: improve queue bypass handling Tejun Heo
2015-07-12 18:00 ` [PATCH 05/10] blkcg: consolidate blkg creation in blkcg_bio_issue_check() Tejun Heo
2015-07-15 22:39   ` [PATCH v2 " Tejun Heo
2015-07-12 18:00 ` [PATCH 06/10] blkcg: add blkg_[rw]stat->aux_cnt and replace cfq_group->dead_stats with it Tejun Heo
2015-07-12 18:00 ` [PATCH 07/10] blkcg: make blkcg_[rw]stat per-cpu Tejun Heo
2015-07-12 18:00 ` [PATCH 08/10] blkcg: make blkg_[rw]stat_recursive_sum() to be able to index into blkcg_gq Tejun Heo
2015-07-12 18:00 ` [PATCH 09/10] blkcg: move io_service_bytes and io_serviced stats " Tejun Heo
2015-07-14 16:09   ` Vivek Goyal
2015-07-15 16:04     ` Tejun Heo
2015-07-15 16:29       ` Vivek Goyal [this message]
2015-07-15 16:53         ` Tejun Heo
2015-07-15 22:40   ` [PATCH v3 " Tejun Heo
2015-07-12 18:00 ` [PATCH 10/10] blkcg: remove cfqg_stats->sectors Tejun Heo
2015-07-16 15:55 ` [PATCH 11/10] blkcg: reduce stack usage of blkg_rwstat_recursive_sum() Tejun Heo

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=20150715162952.GA28818@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=avanzini.arianna@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /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