Apologies for taking so long to reply. Comments in line.
On Tuesday15 Sep, at 11:46 PM, Jason King wrote:
> To recap, the idea is to associate stability levels to kstats
> analogous to the stability levels assigned to dtrace probes.
>
> Defined stability levels:
> KSTAT_STABILITY_PRIVATE
> KSTAT_STABILITY_VOLATILE
> KSTAT_STABILITY_UNCOMITTED
> KSTAT_STABILITY_COMITTED
>
> The current intention is to repurpose the ks_resv field of kstat_t to
> hold this data.
Peter Tribble's comments have encouraged me to think of stability as a
binary quality, perhaps in terms of public/private or stable/
unstable. If this is the case, I would suggest introducing a new
ks_flag bit field:
#define KSTAT_FLAG_STABLE 0x40
This would reduce the complexity of the case by not changing the
kstat_create(9f) interface since there is already a flag argument.
I'm still not convinced that thinking of stability as binary is the
right thing to do, but I have doubts to the utility of the multiple
levels of stability proposed. Also it would appear from John Levon's
comments that the proposed nomenclature is opposite of current use
( DTrace vs current ARC stability nomenclature ). Of the two, I
would pick DTrace just so we could ride on their coat-tails, as it were.
As a kstat-consumer, I am hard pressed to think of situations where I
would make a differentiation any more sophisticated that stable/
unstable. I would be interested to hear other people's thoughts about
this.
> The outstanding questions I'd like to try to get some feedback (and
> hopeful closure) on (and a +1 for eventual submission of an ARC case):
>
> - Should the stability of the kstat name (i.e. it's existance) be
> separate from the data it's presenting (i.e. there should each have
> it's own stability)?
I think the stability quality should be associated with the
(module,name) tuple. Having separate stability values for the kstat
"name" and the kstat data seems an unnecessary distinction. Since
statistics can only be accessed via a kstat tuple ( module, instance,
name ) I don't see any specific reason to give stability levels to
statistics.
> - How should the kstat be assigned the stability value?
> Options:
> 1. A new function is called to set it after kstat_create and
> before kstat_install, kstats that don't do this will default to
> KSTAT_STABILITY_PRIVATE. Thus bumping up the stability level will
> require source code changes.
> 2. The existing kstat_create could be extended to add the
> info. The downside is that I expect it would be messy -- the current
> flags parameter is uchar_t, so it doesn't leave much room for
> expansion. A bit could be set to signal the existence of an
> additional function argument, but that might get a bit messy in terms
> of the impact on the source
> 3. A new function is created that would be used in lieu of
> kstat_create that includes an explicit parameter for the stability.
> Kstats whose stability is something other than private must use this
> function (those that are private could use it or the existing
> kstat_create).
While I have not inspected every kstat_create(9f) call in the source,
a basic pattern for modules which export collections of named kstats
( the majority of exported statistics ) goes something like this:
struct {
kstat_named_t statOne;
kstat_named_t statTwo;
...
} myStatistics;
size_t theSize = sizeof(myStatistics)/sizeof(kstat_named_t);
uchar_t theFlags = KSTAT_FLAG_VIRTUAL;
kstat_t *myKstat = kstat_create
(module,instance,name,class,KSTAT_TYPE_NAMED,theSize,theFlags);
myKstat->ks_data = myStatistics;
kstat_install(myKstat);
If stability were agreed to be a binary quality, the impact on this
pattern is minimal:
uchar_t theFlags = KSTAT_FLAG_VIRTUAL | KSTAT_FLAG_STABLE;
and the rest stays exactly as it was.
Otherwise, I would rather see two new functions added to set and get
stability information ( still talking kernel interfaces ) which I
believe Jason has mentioned before, but bears repeating again:
int kstat_set_stability(kstat_t *ksp,kstat_stability_t stability);
kstat_stability_t kstat_get_stability(kstat_t *ksp)
kstat_create(9F) would be modified to produce kstats whose default
stability is the base level ( whatever that is determined to be ) and
the module author would adjust the stability explicitly before
installing the kstat. I would be in favor of the setter only
modifying kstat's whose KSTAT_FLAG_INVALID bit is set, so as to only
act on kstats that were not presently "active" in the kstat_chain.
I am not against creating a kstat_create_ex(9F) interface which
kstat_create(9F) calls, but it seems unnecessary for the most part if
only to avoid calling the proposed kstat_set_stability() function.
> - What would the process be to increase the stability of a given
> kstat? I suspect some sort of ARC review, but then this gets outside
> of my realm of experience. I suspect a few emails to opensolaris-arc
> or such might prove useful, though I'm not sure if now is the
> appropriate time or not. I'll talk offline to a couple of people and
> see what they recommend.
I think that it would become a part of the ARC process in much the
same manner as interface stability is specified today. Some "Big
Rules" about kstat stability would need to be established and
published so as to promulgate consistent stability use and consumption.
Will you propose to make any changes to libkstat as a part of this
project or leave it for another day?
I think it could either way, and probably would encourage you to leave
it for another day so we can learn what works before it's "set it
stone".
I would definitely give this project a +1
---
erik.oshaughnessy at sun.com - SAE Austin - 512-401-1070
