Re: [ndctl PATCH v2] ndctl: add an api for getting the ars_status overflow flag
On Wed, 2018-06-06 at 13:00 -0700, Dan Williams wrote:
> On Wed, Jun 6, 2018 at 12:53 PM, Verma, Vishal L
> wrote:
> > On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote:
> > > On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma > > 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
> > > > Cc: Dan Williams
> > > > Signed-off-by: Vishal Verma
> > > > ---
> > > > 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.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH v2] ndctl: add an api for getting the ars_status overflow flag
On Wed, Jun 6, 2018 at 12:53 PM, Verma, Vishal L
wrote:
> On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote:
>> On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma
>> 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
>> > Cc: Dan Williams
>> > Signed-off-by: Vishal Verma
>> > ---
>> > 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.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH v2] ndctl: add an api for getting the ars_status overflow flag
On Wed, 2018-06-06 at 12:51 -0700, Dan Williams wrote:
> On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma
> 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
> > Cc: Dan Williams
> > Signed-off-by: Vishal Verma
> > ---
> > 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?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH v2] ndctl: add an api for getting the ars_status overflow flag
On Wed, Jun 6, 2018 at 11:26 AM, Vishal Verma 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
> Cc: Dan Williams
> Signed-off-by: Vishal Verma
> ---
> 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?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
[ndctl PATCH v2] ndctl: add an api for getting the ars_status overflow flag
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
Cc: Dan Williams
Signed-off-by: Vishal Verma
---
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);
+}
+
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
--
2.17.0
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
