Thanks for this taking time to look into this Dan,
Agree with the points you have made earlier that I am summarizing below:
* This is better done in ndctl rather than ipmctl.
* Should only expose general performance metrics and not performance
counters. Performance counter should be exposed via perf
* Vendor specific metrics to be separated from generic performance
metrics.
One way to split generic and vendor specific metrics might be to report
generic performance metrics together with dimm health metrics such as
"temprature_celsius" or "spares_percentage" that are already reported in
by dimm health output.
Vendor specific performance metrics can be reported as a seperate object
in the json output. Something similar to output below:
# ndctl list -DH --stats --vendor-stats
[
{
"dev":"nmem0",
"health":{
"health_state":"ok",
"shutdown_state":"clean",
"temperature_celsius":48.00,
"spares_percentage":10,
/* Generic performance metrics/stats */
"TotalMediaReads": 18929,
"TotalMediaWrites": 0,
....
}
/* Vendor specific stats for the dimm */
"vendor-stats": {
"Controller Reset Count":10
"Controller Reset Elapsed Time": 3600
"Power-on Seconds": 3600
}
}
]
Dan Williams <[email protected]> writes:
> On Wed, May 27, 2020 at 12:24 PM Dan Williams <[email protected]>
> wrote:
> [..]
>> > This was done by adding two new dimm-ops callbacks that were
>> > implemented by the papr_scm implementation within libndctl. These
>> > callbacks are invoked by newly introduce code in 'util/json-smart.c'
>> > that format the returned stats from these new dimm-ops and transform
>> > them into a json-object to later presentation. I would request you to
>> > look at RFC patch-set[2] to understand the implementation details.
>>
>> I'm ok to add some stats to ndctl, but I want ndctl to be limited to
>> general statistics and not performance counters. Performance counters
>> and performance events should be abstracted through perf where
>> possible.
>
> Another aspect that helps common statistics is to expose them in
> sysfs. I'm going to go review your proposed ioctl mechanism, but I
> would hope that is reserved for multi-field command payloads that need
> to be sent as a unit rather than statistics retrieval that is amenable
> to a sysfs interface.
The patchset is using a machenism similar to GET_CONFIG_SIZE/DATA to
retrive a struct composed of tuples of (stat-id, stat-value) from
papr_scm and then exposes them to ndctl via some new dimm-ops.
--
Cheers
~ Vaibhav
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
