On Tue, Sep 13, 2016 at 10:41 AM, Brian Boylston <brian.boyls...@hpe.com> wrote:
> Add a layer of indirection for the ndctl_cmd_smart*() family of
> interfaces. This will allow the underlying implementation to be
> switched based on the DSM family supported by the DIMM.
>
> Cc: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Brian Boylston <brian.boyls...@hpe.com>
> ---
> ndctl/lib/libndctl-private.h | 21 ++++++++
> ndctl/lib/libndctl-smart.c | 111
> ++++++++++++++++++++++++++++++++++---------
> ndctl/lib/libndctl.c | 8 ++++
> ndctl/libndctl.h.in | 1 +
> 4 files changed, 118 insertions(+), 23 deletions(-)
Looks good to me and passes my regression tests.
Just one minor comment below:
>
> diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h
> index 65ef86d..8d2ebfc 100644
> --- a/ndctl/lib/libndctl-private.h
> +++ b/ndctl/lib/libndctl-private.h
> @@ -201,6 +201,27 @@ struct ndctl_cmd {
> };
> };
>
> +struct ndctl_smart_ops {
> + struct ndctl_cmd *(*new_smart)(struct ndctl_dimm *);
> + unsigned int (*smart_get_flags)(struct ndctl_cmd *);
> + unsigned int (*smart_get_health)(struct ndctl_cmd *);
> + unsigned int (*smart_get_temperature)(struct ndctl_cmd *);
> + unsigned int (*smart_get_spares)(struct ndctl_cmd *);
> + unsigned int (*smart_get_alarm_flags)(struct ndctl_cmd *);
> + unsigned int (*smart_get_life_used)(struct ndctl_cmd *);
> + unsigned int (*smart_get_shutdown_state)(struct ndctl_cmd *);
> + unsigned int (*smart_get_vendor_size)(struct ndctl_cmd *);
> + unsigned char *(*smart_get_vendor_data)(struct ndctl_cmd *);
> + struct ndctl_cmd *(*new_smart_threshold)(struct ndctl_dimm *);
> + unsigned int (*smart_threshold_get_alarm_control)(struct ndctl_cmd *);
> + unsigned int (*smart_threshold_get_temperature)(struct ndctl_cmd *);
> + unsigned int (*smart_threshold_get_spares)(struct ndctl_cmd *);
> +};
> +
> +#if HAS_SMART == 1
> +struct ndctl_smart_ops intel_smart_ops;
> +#endif
> +
> /* internal library helpers for conditionally defined command numbers */
> #ifdef HAVE_NDCTL_ARS
> static const int nd_cmd_ars_status = ND_CMD_ARS_STATUS;
> diff --git a/ndctl/lib/libndctl-smart.c b/ndctl/lib/libndctl-smart.c
> index cba1e9d..a172541 100644
> --- a/ndctl/lib/libndctl-smart.c
> +++ b/ndctl/lib/libndctl-smart.c
> @@ -16,7 +16,55 @@
> #include <ndctl/libndctl.h>
> #include "libndctl-private.h"
>
> -NDCTL_EXPORT struct ndctl_cmd *ndctl_dimm_cmd_new_smart(struct ndctl_dimm
> *dimm)
> +/*
> + * The smart_dimm_op() and smart_cmd_op() macros are used here to
> + * define the wrappers around the ndctl_smart_ops:
> + */
> +
> +#define smart_dimm_op(name, op) \
> +NDCTL_EXPORT struct ndctl_cmd *name( \
> + struct ndctl_dimm *dimm) \
> +{ \
> + struct ndctl_smart_ops *ops = ndctl_dimm_get_smart_ops(dimm); \
> + if (ops && ops->op) \
> + return ops->op(dimm); \
> + else \
> + return NULL; \
> +}
> +
> +smart_dimm_op(ndctl_dimm_cmd_new_smart, new_smart)
> +smart_dimm_op(ndctl_dimm_cmd_new_smart_threshold, new_smart_threshold)
There's only two of these routines and I don't suppose there will need
to have many more variants in the future. Let's just open code them
and not use a macro. The rest looks good.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
