Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Apr 2013 02:28:46 +0200
From:      Luigi Rizzo <rizzo@iet.unipi.it>
To:        Pawel Jakub Dawidek <pjd@FreeBSD.org>
Cc:        arch@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>
Subject:   Re: [CFR][CFT] counter(9): new API for faster and raceless counters
Message-ID:  <20130403002846.GB15334@onelab2.iet.unipi.it>
In-Reply-To: <20130402232606.GC1810@garage.freebsd.pl>
References:  <20130401115128.GZ76816@FreeBSD.org> <20130402232606.GC1810@garage.freebsd.pl>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Apr 03, 2013 at 01:26:07AM +0200, Pawel Jakub Dawidek wrote:
> On Mon, Apr 01, 2013 at 03:51:28PM +0400, Gleb Smirnoff wrote:
> >   Hi!
> > 
> >   Together with Konstantin Belousov (kib@) we developed a new API that is
> > initially purposed for (but not limited to) collecting statistical
> > data in kernel.
> 
> Is there any plan to implement universal way of exporting those
> statistics out of the kernel?
> 
> Solaris has a framework for in-kernel statistics, which are exported via
> kstat tool. For ZFS I export them via sysctl. If you have ZFS loaded you
> can try 'sysctl kstat'.
> 
> It would be nice for counter_u64_alloc() to take additional argument
> 'name' and to create sysctl for the counter automatically. We could then
> slowly start migrating userland tools to use sysctls (or some wrapper
> userland API), but we immediately make those statistics available for
> use in scripts.

that is an interesting idea but i believe it can be effectively
built as a wrapper on top of the counter_u64_alloc() routine:

	name_counter(counter_t c, const char *fmt, ...);
	free_named_counter(counter_t c);

After all the name->counter mapping is unidirectional,
and possibly not even necessary on every single counter
(think of ipfw dynamic rules, created on packet arrivals,  so
the counter alloc/dealloc needs to be fast).

It might be useful for the name_counter() routine to support
a printf-style argument to make it easy to build names.

> > o Tiny API for counter(9):
> > 
> >      counter_u64_t
> >      counter_u64_alloc(int wait);
> > 
> >      void
> >      counter_u64_free(counter_u64_t cnt);
> > 
> >      void
> >      counter_u64_add(counter_u64_t cnt, uint64_t inc);
> > 
> >      uint64_t
> >      counter_u64_fetch(counter_u64_t cnt);
> 
> Do you really expect other types in the future? If so, could we at least
> create generic counter_t that internally keeps the type?

I read the u64 in the name mostly as a reminder to users
of the counter size. 

It might actually make sense is to change the type to s64.
This way we could have counters that go negative,
and also use them to accumulate sbintime_t values.

But otherwise i am not sure that we want other types.

u32/s32 might save atomic/critical_enter ops on some archs,
but they saturate so quickly that probably are a bad idea.
And 63/64 bits are quite large already.

	cheers
	luigi



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130403002846.GB15334>