From: Dexuan Cui
Sent: Thursday, March 21, 2019 7:09 PM
IMO there are 2 issues in ndctl/monitor.c: filter_dimm():
1. IMO the cmd numbers ND_CMD_SMART (1) and
ND_CMD_SMART_THRESHOLD(2) are not really device-neutral. They
work for ndctl/lib/intel.c and it looks they happen to work for
ndctl/lib/hpe1.c, as hpe "happens" to have:
NDN_HPE1_CMD_SMART = 1,
NDN_HPE1_CMD_SMART_THRESHOLD = 2,
But for ndctl/lib/msft.c, 1 and 2 mean different things. See .
So the current "ndctl monitor" can't support msft.c.
For ndctl/lib/hyperv.c, 1 and 2 means:
ND_HYPERV_CMD_GET_HEALTH_INFO = 1,
ND_HYPERV_CMD_GET_SHUTDOWN_INFO = 2,
They're also incompatible with ND_CMD_SMART and
Of source, the current code can "work" since these ND_HYPERV_CMD*
numbers happen to be the same as ND_CMD_SMART and
ND_CMD_SMART_THRESHOLD. So this may not be an isue for hyperv.c.
Actually, this _is_ an issue for NVDIMM_FAMILY_HYPERV (and the other
families except for NVDIMM_FAMILY_INTEL) : see the kernel function
acpi_nfit_register_dimms(), where ND_CMD_SMART is set in the
"cmd_mask" only for NVDIMM_FAMILY_INTEL.
So, on Hyper-V, ndctl_dimm_is_cmd_supported(dimm, ND_CMD_SMART)
is always false, and "ndctl monitor" exits with "no smart support".
2. The current code assumes ND_SMART_ALARM_VALID must be supported.
However, NVDIMM_FAMILY_HYPERV doesn't support it, so currently
ndctl/monitor.c: filter_dimm() reports an error and returns -- for Hyper-V,
we should not return... hence I made this patch.
Of source, we can add a new op to dimm->ops, e.g.
ops->monitor_supported() and then change the common code accordingly,
but I can imagine that would require a lot more changes and I guess it may not
worth that effort? Please share your insights!
Looking forward to your suggestion!