Responses inline :) On Wed, Apr 15, 2009 at 11:22 PM, Erik O'Shaughnessy <erik.oshaughne...@sun.com> wrote: > Jason, > > I am interested in this happening, so I thought I would at least comment. > > Some of my questions are more implementation detail specific, but I hope > they should incite some more dialog about your proposal ;) > > Good stuff, questions/comments in-line > > -ejo > > On 7 Apr, at 2:25 PM, Jason King wrote: >> >> Stability attributes: >> >> The thinking is unless there's a good reason to diverge, for >> consistency (and sanity of someone trying to figure out what to use), >> I think keeping it analogous to the dtrace attributes makes the most >> sense: >> >> typedef uint8_t kstat_stability_t; >> >> KSTAT_STABILITY_INTERNAL internal to kstat system >> KSTAT_STABILITY_PRIVATE private to ON, the default for all >> kstats >> KSTAT_STABILITY_OBSOLETE scheduled for removal >> KSTAT_STABILITY_EXTERNAL maintained by a 3rd party >> KSTAT_STABILITY_UNSTABLE new or rapidly changing >> KSTAT_STABILITY_EVOLVING less rapidly changing >> KSTAT_STABILITY_STABLE mature interface >> KSTAT_STABILITY_STANDARD industry standard >> KSTAT_STABILITY_MAX maximum valid stability >> >> >> Kernel Interfaces: >> >> int kstat_set_stability(const char *ks_module, const char *ks_name, >> kstat_stability_t stability); >> Desc: set the kstat to a given stability level >> Returns: 0 - success >> EINVAL - stability level isn't valid >> ENOENT - kstat doesn't exist >> >> ??: should this be mutable, or should it be allowed to only be set >> once, and return an error from there on out? > > My intuition says that it should be immutable but I can think of (contrived) > situations where the stability level might be changed after the fact. > > Looking at the interface signature, I wonder why you chose to specify the > kstat using module and name? Will the stability be added to the > kstat_t structure or will it be in an auxiliary dictionary separate from the > kernel kstat chain? > > If it's not separate, it makes more sense to me to modify kstat_create() to > take a new argument. Oh great, I just realized why that won't fly. The > advertised stability of kstat_create() probably won't allow us to modify it > all willy nilly ;) Never mind. > > I still have the feeling the signature should be something like: > > int kstat_set_stability(kstat_t *ks,kstat_stability_t stability)
Well one thing I wasn't sure about -- that means each instance could potentially have it's own stability. I'm not a kstat expert, but for foo:0:stat to have one stability level and foo:1:stat to have a different stability seems odd to me. I would expect foo:*:stat to be the same. But if my expectations are wrong, please set me straight :) > > The thinking here is that authors will create kstats first and then tweak > the stability. That was my thought as well -- it would return ENOENT if you tried to set the stability before creating the stat. > >> int kstat_get_stability(const char *ks_module, const char *ks_name, >> kstat_stability_t *stability); >> Returns: 0 - stability value in stability. If the stability value was >> not explicitly set, *stability contains KSTAT_STABILITY_PRIVATE. >> ENOENT - kstat does not exist. >> >> User interfaces: >> >> int kstat_get_stability(kstat_ctl_t *kc, const char *ks_module, const >> char *ks_name, kstat_stability_t *stability) >> Similar to kernel interface >> Returns 0 on success, -1 on failure. On failure, errno is set to: >> ENOENT - kstat does not exist. kstat_t > > My concern here is if the stability information will reside in the structure > or in an auxiliary kernel data structure. If it is in the kstat then it > gets copied into user space when the rest of the kstat_t is copied ( via an > IOCTL ). If it is in a new data structure, we may need a new IOCTL in the > kstat device driver to accommodate. It could be copied with the IOCTL, but it looks like the userland struct is not opaque, so I'm concerned appending fields could break things, though I might need to look a little closer to see if it's a valid concern. As a bit of background, this is really just to clear the road to tag a few (~10-15) kstats with something better than 'private' so that they can be used by another module I'm working on (snmp module w/ some perf counters -- if you want more details, please read the permib-dev at opensolaris.org archives). > >> ??: How should the perl module be updated. > > > Sounds like we add a command line switch to kstat(3perl) to filter on > stability and teach libkstat(3perl) about the new stability user land > interfaces. > > I don't think it should be especially difficult. > > > --- > erik.oshaughne...@sun.com:Strategic Applications Engineering, Austin > Texas:512-401-1070 > > _______________________________________________ perf-discuss mailing list perf-discuss@opensolaris.org
