| Message ID | 20180606182605.28670-1-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma <vishal.l.verma@intel.com> wrote: > The ARS status command defines a 'flags' field that wasn't being exposed > via an API yet. Since there is only one flag defined in ACPI 6.2, add a > helper for retrieving it (overflow flag). > > Reported-by: Jacek Zloch <jacek.zloch@intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > ndctl/lib/ars.c | 11 +++++++++++ > ndctl/lib/libndctl.sym | 1 + > ndctl/libndctl.h | 4 ++++ > 3 files changed, 16 insertions(+) > > v2: instead of exposing the binary representation of flags, provide a > helper for each flag. ACPI currently defines a single 'overflow' flag. > (Dan) > > diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c > index e04b51e..b765c88 100644 > --- a/ndctl/lib/ars.c > +++ b/ndctl/lib/ars.c > @@ -269,6 +269,17 @@ NDCTL_EXPORT unsigned long long ndctl_cmd_ars_get_record_len( > return ars_stat->ars_status->records[rec_index].length; > } > > +NDCTL_EXPORT int ndctl_cmd_ars_stat_get_flag_overflow( > + struct ndctl_cmd *ars_stat) > +{ > + struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(ars_stat)); > + > + if (!validate_ars_stat(ctx, ars_stat)) > + return -EINVAL; > + > + return (ars_stat->ars_status->flags & ND_ARS_STAT_FLAG_OVERFLOW); > +} How about return bool since it's a flag?
On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote: > On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma <vishal.l.verma@intel.com> > wrote: > > The ARS status command defines a 'flags' field that wasn't being > > exposed > > via an API yet. Since there is only one flag defined in ACPI 6.2, add a > > helper for retrieving it (overflow flag). > > > > Reported-by: Jacek Zloch <jacek.zloch@intel.com> > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > ndctl/lib/ars.c | 11 +++++++++++ > > ndctl/lib/libndctl.sym | 1 + > > ndctl/libndctl.h | 4 ++++ > > 3 files changed, 16 insertions(+) > > > > v2: instead of exposing the binary representation of flags, provide a > > helper for each flag. ACPI currently defines a single 'overflow' flag. > > (Dan) > > > > diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c > > index e04b51e..b765c88 100644 > > --- a/ndctl/lib/ars.c > > +++ b/ndctl/lib/ars.c > > @@ -269,6 +269,17 @@ NDCTL_EXPORT unsigned long long > > ndctl_cmd_ars_get_record_len( > > return ars_stat->ars_status->records[rec_index].length; > > } > > > > +NDCTL_EXPORT int ndctl_cmd_ars_stat_get_flag_overflow( > > + struct ndctl_cmd *ars_stat) > > +{ > > + struct ndctl_ctx *ctx = > > ndctl_bus_get_ctx(cmd_to_bus(ars_stat)); > > + > > + if (!validate_ars_stat(ctx, ars_stat)) > > + return -EINVAL; > > + > > + return (ars_stat->ars_status->flags & > > ND_ARS_STAT_FLAG_OVERFLOW); > > +} > > How about return bool since it's a flag? I considered it, but int allows us to return an error for an invalid status command. If we use a bool, should we just return 'false' for a bad command?
On Wed, Jun 6, 2018 at 12:53 PM, Verma, Vishal L <vishal.l.verma@intel.com> wrote: > On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote: >> On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma <vishal.l.verma@intel.com> >> wrote: >> > The ARS status command defines a 'flags' field that wasn't being >> > exposed >> > via an API yet. Since there is only one flag defined in ACPI 6.2, add a >> > helper for retrieving it (overflow flag). >> > >> > Reported-by: Jacek Zloch <jacek.zloch@intel.com> >> > Cc: Dan Williams <dan.j.williams@intel.com> >> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >> > --- >> > ndctl/lib/ars.c | 11 +++++++++++ >> > ndctl/lib/libndctl.sym | 1 + >> > ndctl/libndctl.h | 4 ++++ >> > 3 files changed, 16 insertions(+) >> > >> > v2: instead of exposing the binary representation of flags, provide a >> > helper for each flag. ACPI currently defines a single 'overflow' flag. >> > (Dan) >> > >> > diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c >> > index e04b51e..b765c88 100644 >> > --- a/ndctl/lib/ars.c >> > +++ b/ndctl/lib/ars.c >> > @@ -269,6 +269,17 @@ NDCTL_EXPORT unsigned long long >> > ndctl_cmd_ars_get_record_len( >> > return ars_stat->ars_status->records[rec_index].length; >> > } >> > >> > +NDCTL_EXPORT int ndctl_cmd_ars_stat_get_flag_overflow( >> > + struct ndctl_cmd *ars_stat) >> > +{ >> > + struct ndctl_ctx *ctx = >> > ndctl_bus_get_ctx(cmd_to_bus(ars_stat)); >> > + >> > + if (!validate_ars_stat(ctx, ars_stat)) >> > + return -EINVAL; >> > + >> > + return (ars_stat->ars_status->flags & >> > ND_ARS_STAT_FLAG_OVERFLOW); >> > +} >> >> How about return bool since it's a flag? > > I considered it, but int allows us to return an error for an invalid status > command. If we use a bool, should we just return 'false' for a bad command? Oh, sorry, missed that. In that case let's do: return !!(ars_stat->ars_status->flags & ND_ARS_STAT_FLAG_OVERFLOW); So it's defined to be 0, 1, or -error.
On Wed, 2018-06-06 at 13:00 -0700, Dan Williams wrote: > On Wed, Jun 6, 2018 at 12:53 PM, Verma, Vishal L > <vishal.l.verma@intel.com> wrote: > > On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote: > > > On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma <vishal.l.verma@intel.c > > > om> > > > wrote: > > > > The ARS status command defines a 'flags' field that wasn't being > > > > exposed > > > > via an API yet. Since there is only one flag defined in ACPI 6.2, > > > > add a > > > > helper for retrieving it (overflow flag). > > > > > > > > Reported-by: Jacek Zloch <jacek.zloch@intel.com> > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > > --- > > > > ndctl/lib/ars.c | 11 +++++++++++ > > > > ndctl/lib/libndctl.sym | 1 + > > > > ndctl/libndctl.h | 4 ++++ > > > > 3 files changed, 16 insertions(+) > > > > > > > > v2: instead of exposing the binary representation of flags, provide > > > > a > > > > helper for each flag. ACPI currently defines a single 'overflow' > > > > flag. > > > > (Dan) > > > > > > > > diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c > > > > index e04b51e..b765c88 100644 > > > > --- a/ndctl/lib/ars.c > > > > +++ b/ndctl/lib/ars.c > > > > @@ -269,6 +269,17 @@ NDCTL_EXPORT unsigned long long > > > > ndctl_cmd_ars_get_record_len( > > > > return ars_stat->ars_status->records[rec_index].length; > > > > } > > > > > > > > +NDCTL_EXPORT int ndctl_cmd_ars_stat_get_flag_overflow( > > > > + struct ndctl_cmd *ars_stat) > > > > +{ > > > > + struct ndctl_ctx *ctx = > > > > ndctl_bus_get_ctx(cmd_to_bus(ars_stat)); > > > > + > > > > + if (!validate_ars_stat(ctx, ars_stat)) > > > > + return -EINVAL; > > > > + > > > > + return (ars_stat->ars_status->flags & > > > > ND_ARS_STAT_FLAG_OVERFLOW); > > > > +} > > > > > > How about return bool since it's a flag? > > > > I considered it, but int allows us to return an error for an invalid > > status > > command. If we use a bool, should we just return 'false' for a bad > > command? > > Oh, sorry, missed that. In that case let's do: > > return !!(ars_stat->ars_status->flags & ND_ARS_STAT_FLAG_OVERFLOW); > > So it's defined to be 0, 1, or -error. Ok, yep that is better, I'll fix and send a new version.
diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c index e04b51e..b765c88 100644 --- a/ndctl/lib/ars.c +++ b/ndctl/lib/ars.c @@ -269,6 +269,17 @@ NDCTL_EXPORT unsigned long long ndctl_cmd_ars_get_record_len( return ars_stat->ars_status->records[rec_index].length; } +NDCTL_EXPORT int ndctl_cmd_ars_stat_get_flag_overflow( + struct ndctl_cmd *ars_stat) +{ + struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(ars_stat)); + + if (!validate_ars_stat(ctx, ars_stat)) + return -EINVAL; + + return (ars_stat->ars_status->flags & ND_ARS_STAT_FLAG_OVERFLOW); +} + NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_clear_error( unsigned long long address, unsigned long long len, struct ndctl_cmd *ars_cap) diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym index c1228e5..e939993 100644 --- a/ndctl/lib/libndctl.sym +++ b/ndctl/lib/libndctl.sym @@ -365,4 +365,5 @@ global: ndctl_cmd_ars_cap_get_clear_unit; ndctl_namespace_inject_error2; ndctl_namespace_uninject_error2; + ndctl_cmd_ars_stat_get_flag_overflow; } LIBNDCTL_15; diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h index be997ac..3d141a6 100644 --- a/ndctl/libndctl.h +++ b/ndctl/libndctl.h @@ -187,6 +187,9 @@ int ndctl_dimm_is_enabled(struct ndctl_dimm *dimm); int ndctl_dimm_disable(struct ndctl_dimm *dimm); int ndctl_dimm_enable(struct ndctl_dimm *dimm); +/* ars_status flags */ +#define ND_ARS_STAT_FLAG_OVERFLOW (1 << 0) + struct ndctl_cmd; struct ndctl_cmd *ndctl_bus_cmd_new_ars_cap(struct ndctl_bus *bus, unsigned long long address, unsigned long long len); @@ -210,6 +213,7 @@ struct ndctl_cmd *ndctl_bus_cmd_new_clear_error(unsigned long long address, unsigned long long ndctl_cmd_clear_error_get_cleared( struct ndctl_cmd *clear_err); unsigned int ndctl_cmd_ars_cap_get_clear_unit(struct ndctl_cmd *ars_cap); +int ndctl_cmd_ars_stat_get_flag_overflow(struct ndctl_cmd *ars_stat); /* * Note: ndctl_cmd_smart_get_temperature is an alias for
The ARS status command defines a 'flags' field that wasn't being exposed via an API yet. Since there is only one flag defined in ACPI 6.2, add a helper for retrieving it (overflow flag). Reported-by: Jacek Zloch <jacek.zloch@intel.com> Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- ndctl/lib/ars.c | 11 +++++++++++ ndctl/lib/libndctl.sym | 1 + ndctl/libndctl.h | 4 ++++ 3 files changed, 16 insertions(+) v2: instead of exposing the binary representation of flags, provide a helper for each flag. ACPI currently defines a single 'overflow' flag. (Dan)