From: Mikulas Patocka <mpatocka@redhat.com>
To: Coly Li <colyli@gmail.com>
Cc: Mike Snitzer <msnitzer@redhat.com>,
Laurence Oberman <loberman@redhat.com>,
Tao Ma <boyu.mt@taobao.com>, Robin Dong <sanbai@alibaba-inc.com>,
dm-devel@redhat.com, "Alasdair G. Kergon" <agk@redhat.com>
Subject: Re: [PATCH 3/4] dm stats: report histogram of latencies
Date: Mon, 15 Jun 2015 09:06:33 -0400 (EDT) [thread overview]
Message-ID: <alpine.LRH.2.02.1506150904280.7275@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <557C6272.8070200@gmail.com>
These micro optimizations would save one or two instructions, so they are
not really needed. Also, these micro optimizations are at a code path that
prints the statistics, not the code path that processes i/o requests.
Mikulas
On Sun, 14 Jun 2015, Coly Li wrote:
> ? 15/6/10 ??5:22, Mikulas Patocka ??:
> > This patch adds an option to dm statistics to collect and report the
> > histogram of latencies.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >
> > ---
> > Documentation/device-mapper/statistics.txt | 21 ++-
> > drivers/md/dm-stats.c | 201 +++++++++++++++++++++++++----
> > 2 files changed, 196 insertions(+), 26 deletions(-)
> >
> > Index: linux-4.1-rc7/drivers/md/dm-stats.c
> > ===================================================================
> > --- linux-4.1-rc7.orig/drivers/md/dm-stats.c 2015-06-08 16:38:43.000000000 +0200
> > +++ linux-4.1-rc7/drivers/md/dm-stats.c 2015-06-08 17:02:01.000000000 +0200
> > @@ -29,6 +29,7 @@ struct dm_stat_percpu {
> > unsigned long long io_ticks[2];
> > unsigned long long io_ticks_total;
> > unsigned long long time_in_queue;
> > + unsigned long long *histogram;
> > };
> >
> > struct dm_stat_shared {
> [snip]
> > @@ -619,6 +700,11 @@ static void __dm_stat_init_temporary_per
> > shared->tmp.io_ticks[WRITE] += ACCESS_ONCE(p->io_ticks[WRITE]);
> > shared->tmp.io_ticks_total += ACCESS_ONCE(p->io_ticks_total);
> > shared->tmp.time_in_queue += ACCESS_ONCE(p->time_in_queue);
> > + if (s->n_histogram_entries) {
> > + unsigned i;
> > + for (i = 0; i < s->n_histogram_entries + 1; i++)
> > + shared->tmp.histogram[i] += ACCESS_ONCE(p->histogram[i]);
>
> s->n_histogram_entries + 1 in for() looping will be calculated many times, maybe this way could be better,
>
> + if (s->n_histogram_entries) {
> + unsigned i, j;
> + j = s->n_histogram_entries + 1;
> + for (i = 0; i < j; i++)
> + shared->tmp.histogram[i] += ACCESS_ONCE(p->histogram[i]);
>
> > + }
> > }
> > }
> >
> > @@ -648,6 +734,15 @@ static void __dm_stat_clear(struct dm_st
> > p->io_ticks_total -= shared->tmp.io_ticks_total;
> > p->time_in_queue -= shared->tmp.time_in_queue;
> > local_irq_enable();
> > + if (s->n_histogram_entries) {
> > + unsigned i;
> > + for (i = 0; i < s->n_histogram_entries + 1; i++) {
> Same situation here,
>
> + unsigned i, j;
> + j = s->n_histogram_entries + 1;
> + for (i = 0; i < j; i++) {
>
> > + local_irq_disable();
> > + p = &s->stat_percpu[smp_processor_id()][x];
> > + p->histogram[i] -= shared->tmp.histogram[i];
> > + local_irq_enable();
> > + }
> > + }
> > }
> > }
> >
> > @@ -737,7 +832,7 @@ static int dm_stats_print(struct dm_stat
> >
> > __dm_stat_init_temporary_percpu_totals(shared, s, x);
> >
> > - DMEMIT("%llu+%llu %llu %llu %llu %llu %llu %llu %llu %llu %d %llu %llu %llu %llu\n",
> > + DMEMIT("%llu+%llu %llu %llu %llu %llu %llu %llu %llu %llu %d %llu %llu %llu %llu",
> > (unsigned long long)start,
> > (unsigned long long)step,
> > shared->tmp.ios[READ],
> > @@ -753,6 +848,13 @@ static int dm_stats_print(struct dm_stat
> > dm_jiffies_to_msec64(s, shared->tmp.time_in_queue),
> > dm_jiffies_to_msec64(s, shared->tmp.io_ticks[READ]),
> > dm_jiffies_to_msec64(s, shared->tmp.io_ticks[WRITE]));
> > + if (s->n_histogram_entries) {
> > + unsigned i;
> > + for (i = 0; i < s->n_histogram_entries + 1; i++) {
> Same situation here,
>
> + unsigned i, j;
> + j = s->n_histogram_entries + 1;
> + for (i = 0; i < j; i++) {
>
> > + DMEMIT("%s%llu", !i ? " " : ":", shared->tmp.histogram[i]);
> > + }
> > + }
> > + DMEMIT("\n");
> >
> > if (unlikely(sz + 1 >= maxlen))
> > goto buffer_overflow;
> [snip]
>
> Coly
>
next prev parent reply other threads:[~2015-06-15 13:06 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-09 21:20 [PATCH 0/4] Integrate dm-latency functionality to dm-statistics Mikulas Patocka
2015-06-09 21:21 ` [PATCH 1/4] dm-statistics: better argument validation Mikulas Patocka
2015-06-09 21:21 ` [PATCH 2/4] dm stats: support precise timestamps Mikulas Patocka
2015-06-10 17:10 ` Mike Snitzer
2015-06-10 17:33 ` Mikulas Patocka
2015-06-10 17:40 ` Mike Snitzer
2015-06-13 17:03 ` Coly Li
2015-06-15 13:04 ` Mikulas Patocka
2015-06-15 14:17 ` Coly Li
2015-06-16 15:33 ` Vivek Goyal
2015-06-16 19:27 ` Mikulas Patocka
2015-06-17 1:43 ` Vivek Goyal
2015-06-17 13:17 ` Mikulas Patocka
2015-06-17 13:20 ` Vivek Goyal
2015-06-17 15:18 ` Bryn M. Reeves
2015-06-17 14:54 ` Bryn M. Reeves
2015-06-17 14:52 ` Bryn M. Reeves
2015-07-27 15:11 ` Bryn M. Reeves
2015-06-09 21:22 ` [PATCH 3/4] dm stats: report histogram of latencies Mikulas Patocka
2015-06-13 17:03 ` Coly Li
2015-06-15 13:06 ` Mikulas Patocka [this message]
2015-06-15 14:41 ` Coly Li
2015-06-15 15:34 ` Mikulas Patocka
2015-06-16 16:21 ` Vivek Goyal
2015-06-09 21:22 ` [PATCH 4/4] dm stats: support statistics on requests-based devices Mikulas Patocka
2015-06-09 21:23 ` Laurence Oberman
2015-06-13 17:02 ` [PATCH 0/4] Integrate dm-latency functionality to dm-statistics Coly Li
2015-06-15 12:47 ` Mikulas Patocka
2015-06-15 14:35 ` Coly Li
2015-06-17 13:22 ` Mikulas Patocka
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=alpine.LRH.2.02.1506150904280.7275@file01.intranet.prod.int.rdu2.redhat.com \
--to=mpatocka@redhat.com \
--cc=agk@redhat.com \
--cc=boyu.mt@taobao.com \
--cc=colyli@gmail.com \
--cc=dm-devel@redhat.com \
--cc=loberman@redhat.com \
--cc=msnitzer@redhat.com \
--cc=sanbai@alibaba-inc.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.