Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 27 Feb 2020 11:10:19 +0900
From:   Masami Hiramatsu <mhiramat@kernel.org>
To:     Luigi Rizzo <lrizzo@google.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        naveen.n.rao@linux.ibm.com, ardb@kernel.org,
        Luigi Rizzo <rizzo@iet.unipi.it>,
        Paolo Abeni <pabeni@redhat.com>, giuseppe.lettieri@unipi.it,
        Toke =?UTF-8?B?SMO4aWxhbmQtSsO4cmdlbnNlbg==?= <toke@redhat.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>, mingo@redhat.com,
        acme@kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v2 1/2] kstats: kernel metric collector
Message-Id: <20200227111019.8fd6f57819282a08ced3ce35@kernel.org>
In-Reply-To: <CAMOZA0LU_mGPre9gsJSZeG19fUjLWb+6xuG8-2yv5gJRHwWzqQ@mail.gmail.com>
References: <20200226134637.31670-1-lrizzo@google.com>
        <20200226134637.31670-2-lrizzo@google.com>
        <20200226161941.GZ18400@hirez.programming.kicks-ass.net>
        <CAMOZA0LU_mGPre9gsJSZeG19fUjLWb+6xuG8-2yv5gJRHwWzqQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Luigi,

On Wed, 26 Feb 2020 11:31:01 -0800
Luigi Rizzo <lrizzo@google.com> wrote:

> On Wed, Feb 26, 2020 at 8:19 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Feb 26, 2020 at 05:46:36AM -0800, Luigi Rizzo wrote:
> > > kstats is a helper to accumulate in-kernel metrics (timestamps, sizes,
> > > etc.) and show distributions through debugfs.
> > > Set CONFIG_KSTATS=m or y to enable it.
> > >
> > > Creating a metric takes one line of code (and one to destroy it):
> > >
> > >   struct kstats *key = kstats_new("foo", 3 /* frac_bits */);
> > >   ...
> > >   kstats_delete(key);
> > >
> > > The following line records a u64 sample:
> > >
> > >   kstats_record(key, value);
> > >
> > > kstats_record() is cheap (5ns hot cache, 250ns cold cache). Samples are
> > > accumulated in a per-cpu array with 2^frac_bits slots for each power
> > > of 2. Using frac_bits = 3 gives about 30 slots per decade.
> >
> > So I think everybody + dog has written code like this, although I never
> > bothered with the log2 based buckets myself. Nor have I ever bothered
> > with doing a debugfs interface.
> 
> the above is perhaps one excellent argument to why it may deserve to be in:
> so that people don't need to write the measurement code time and again,
> or, as I have done myself multiple times, use some inferior hack (racy
> counter, coarse buckets) or give up measuring things and rely on guessing.
> 
> > I find it very hard to convince myself something like this deserves to
> > live upstream, vs. remaining in the local debug/hack toolbox.
> >
> > Tracing has an aggregator (histogram), you can dump the raw deltas, or
> > you can hack up a custom aggregator in a few lines, or you do BPF if
> > you're so inclined.
> 
> And this is possibly another good argument: sometimes the systems where it
> would be interesting to collect data are not accessible to the developers with
> skills to write the monitoring code and run a modified kernel.

Would you mean the histogram requires more skills to use it?
I think Peter's point is that there is already an "ability" and "interface"
to do that in kenrel. If you think that is not easy to use, can you modify
existing features or add a user tool to use it easier?

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>