diff mbox series

[ndctl,v2,3/6] libcxl: return the partition alignment field in bytes

Message ID ca1821eee9f8e2372e378165d5c24bbf9161e6fe.1641965853.git.alison.schofield@intel.com
State Superseded
Headers show
Series Add partitioning support for CXL memdevs | expand

Commit Message

Alison Schofield Jan. 12, 2022, 6:33 a.m. UTC
From: Alison Schofield <alison.schofield@intel.com>

Per the CXL specification, the partition alignment field reports
the alignment value in multiples of 256MB. In the libcxl API, values
for all capacity fields are defined to return bytes.

Update the partition alignment accessor to return bytes so that it
is in sync with other capacity related fields.

Rename the function to reflect the new return value:
cxl_cmd_identify_get_partition_align_bytes()

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/lib/libcxl.c   | 4 ++--
 cxl/lib/libcxl.sym | 2 +-
 cxl/libcxl.h       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Comments

Alison Schofield Jan. 13, 2022, 4:31 p.m. UTC | #1
On Tue, Jan 11, 2022 at 10:33:31PM -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> Per the CXL specification, the partition alignment field reports
> the alignment value in multiples of 256MB. In the libcxl API, values
> for all capacity fields are defined to return bytes.
> 
> Update the partition alignment accessor to return bytes so that it
> is in sync with other capacity related fields.
> 
> Rename the function to reflect the new return value:
> cxl_cmd_identify_get_partition_align_bytes()

Vishal,
Just realized that the cxl_identify_get_partition_align() API was released
in ndctl-v72. Does that mean NAK on changing the name here?
Alison

> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  cxl/lib/libcxl.c   | 4 ++--
>  cxl/lib/libcxl.sym | 2 +-
>  cxl/libcxl.h       | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 1fd584a..823bcb0 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1078,7 +1078,7 @@ CXL_EXPORT int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev,
>  	return 0;
>  }
>  
> -CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
> +CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align_bytes(
>  		struct cxl_cmd *cmd)
>  {
>  	struct cxl_cmd_identify *id =
> @@ -1089,7 +1089,7 @@ CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
>  	if (cmd->status < 0)
>  		return cmd->status;
>  
> -	return le64_to_cpu(id->partition_align);
> +	return le64_to_cpu(id->partition_align) * CXL_CAPACITY_MULTIPLIER;
>  }
>  
>  CXL_EXPORT unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd)
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index b7e969f..1e2cf05 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -34,7 +34,7 @@ global:
>  	cxl_cmd_identify_get_total_bytes;
>  	cxl_cmd_identify_get_volatile_only_bytes;
>  	cxl_cmd_identify_get_persistent_only_bytes;
> -	cxl_cmd_identify_get_partition_align;
> +	cxl_cmd_identify_get_partition_align_bytes;
>  	cxl_cmd_identify_get_label_size;
>  	cxl_cmd_new_get_health_info;
>  	cxl_cmd_health_info_get_maintenance_needed;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 46f99fb..f19ed4f 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -71,7 +71,7 @@ int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev, int fw_len);
>  unsigned long long cxl_cmd_identify_get_total_bytes(struct cxl_cmd *cmd);
>  unsigned long long cxl_cmd_identify_get_volatile_only_bytes(struct cxl_cmd *cmd);
>  unsigned long long cxl_cmd_identify_get_persistent_only_bytes(struct cxl_cmd *cmd);
> -unsigned long long cxl_cmd_identify_get_partition_align(struct cxl_cmd *cmd);
> +unsigned long long cxl_cmd_identify_get_partition_align_bytes(struct cxl_cmd *cmd);
>  unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd);
>  struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
>  int cxl_cmd_health_info_get_maintenance_needed(struct cxl_cmd *cmd);
> -- 
> 2.31.1
>
Alison Schofield Jan. 13, 2022, 4:34 p.m. UTC | #2
On Thu, Jan 13, 2022 at 08:31:49AM -0800, Alison Schofield wrote:
> 
> On Tue, Jan 11, 2022 at 10:33:31PM -0800, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > Per the CXL specification, the partition alignment field reports
> > the alignment value in multiples of 256MB. In the libcxl API, values
> > for all capacity fields are defined to return bytes.
> > 
> > Update the partition alignment accessor to return bytes so that it
> > is in sync with other capacity related fields.
> > 
> > Rename the function to reflect the new return value:
> > cxl_cmd_identify_get_partition_align_bytes()
> 
> Vishal,
> Just realized that the cxl_identify_get_partition_align() API was released
> in ndctl-v72. Does that mean NAK on changing the name here?
> Alison

That question was incomplete! Does that mean NAK on changing what it
returns too?  Or are 'we' early enough in cxl-cli to make such a change?

> 
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  cxl/lib/libcxl.c   | 4 ++--
> >  cxl/lib/libcxl.sym | 2 +-
> >  cxl/libcxl.h       | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index 1fd584a..823bcb0 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -1078,7 +1078,7 @@ CXL_EXPORT int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev,
> >  	return 0;
> >  }
> >  
> > -CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
> > +CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align_bytes(
> >  		struct cxl_cmd *cmd)
> >  {
> >  	struct cxl_cmd_identify *id =
> > @@ -1089,7 +1089,7 @@ CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
> >  	if (cmd->status < 0)
> >  		return cmd->status;
> >  
> > -	return le64_to_cpu(id->partition_align);
> > +	return le64_to_cpu(id->partition_align) * CXL_CAPACITY_MULTIPLIER;
> >  }
> >  
> >  CXL_EXPORT unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd)
> > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > index b7e969f..1e2cf05 100644
> > --- a/cxl/lib/libcxl.sym
> > +++ b/cxl/lib/libcxl.sym
> > @@ -34,7 +34,7 @@ global:
> >  	cxl_cmd_identify_get_total_bytes;
> >  	cxl_cmd_identify_get_volatile_only_bytes;
> >  	cxl_cmd_identify_get_persistent_only_bytes;
> > -	cxl_cmd_identify_get_partition_align;
> > +	cxl_cmd_identify_get_partition_align_bytes;
> >  	cxl_cmd_identify_get_label_size;
> >  	cxl_cmd_new_get_health_info;
> >  	cxl_cmd_health_info_get_maintenance_needed;
> > diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> > index 46f99fb..f19ed4f 100644
> > --- a/cxl/libcxl.h
> > +++ b/cxl/libcxl.h
> > @@ -71,7 +71,7 @@ int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev, int fw_len);
> >  unsigned long long cxl_cmd_identify_get_total_bytes(struct cxl_cmd *cmd);
> >  unsigned long long cxl_cmd_identify_get_volatile_only_bytes(struct cxl_cmd *cmd);
> >  unsigned long long cxl_cmd_identify_get_persistent_only_bytes(struct cxl_cmd *cmd);
> > -unsigned long long cxl_cmd_identify_get_partition_align(struct cxl_cmd *cmd);
> > +unsigned long long cxl_cmd_identify_get_partition_align_bytes(struct cxl_cmd *cmd);
> >  unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd);
> >  struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
> >  int cxl_cmd_health_info_get_maintenance_needed(struct cxl_cmd *cmd);
> > -- 
> > 2.31.1
> > 
>
Verma, Vishal L Jan. 14, 2022, 7:28 a.m. UTC | #3
On Thu, 2022-01-13 at 08:34 -0800, Alison Schofield wrote:
> On Thu, Jan 13, 2022 at 08:31:49AM -0800, Alison Schofield wrote:
> > 
> > On Tue, Jan 11, 2022 at 10:33:31PM -0800, alison.schofield@intel.com wrote:
> > > From: Alison Schofield <alison.schofield@intel.com>
> > > 
> > > Per the CXL specification, the partition alignment field reports
> > > the alignment value in multiples of 256MB. In the libcxl API, values
> > > for all capacity fields are defined to return bytes.
> > > 
> > > Update the partition alignment accessor to return bytes so that it
> > > is in sync with other capacity related fields.
> > > 
> > > Rename the function to reflect the new return value:
> > > cxl_cmd_identify_get_partition_align_bytes()
> > 
> > Vishal,
> > Just realized that the cxl_identify_get_partition_align() API was released
> > in ndctl-v72. Does that mean NAK on changing the name here?
> > Alison
> 
> That question was incomplete! Does that mean NAK on changing what it
> returns too?  Or are 'we' early enough in cxl-cli to make such a change?

Ah yep - changing the name is definitely a no go. I think changing what
it returns is okay - I see it as a bug fix. The least-surprise return
is bytes, and returning anything else was a bug. Luckily we don't need
to change the return type :)

> 
> > 
> > > 
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> > >  cxl/lib/libcxl.c   | 4 ++--
> > >  cxl/lib/libcxl.sym | 2 +-
> > >  cxl/libcxl.h       | 2 +-
> > >  3 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > > index 1fd584a..823bcb0 100644
> > > --- a/cxl/lib/libcxl.c
> > > +++ b/cxl/lib/libcxl.c
> > > @@ -1078,7 +1078,7 @@ CXL_EXPORT int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev,
> > >  	return 0;
> > >  }
> > >  
> > > -CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
> > > +CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align_bytes(
> > >  		struct cxl_cmd *cmd)
> > >  {
> > >  	struct cxl_cmd_identify *id =
> > > @@ -1089,7 +1089,7 @@ CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
> > >  	if (cmd->status < 0)
> > >  		return cmd->status;
> > >  
> > > -	return le64_to_cpu(id->partition_align);
> > > +	return le64_to_cpu(id->partition_align) * CXL_CAPACITY_MULTIPLIER;
> > >  }
> > >  
> > >  CXL_EXPORT unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd)
> > > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > > index b7e969f..1e2cf05 100644
> > > --- a/cxl/lib/libcxl.sym
> > > +++ b/cxl/lib/libcxl.sym
> > > @@ -34,7 +34,7 @@ global:
> > >  	cxl_cmd_identify_get_total_bytes;
> > >  	cxl_cmd_identify_get_volatile_only_bytes;
> > >  	cxl_cmd_identify_get_persistent_only_bytes;
> > > -	cxl_cmd_identify_get_partition_align;
> > > +	cxl_cmd_identify_get_partition_align_bytes;
> > >  	cxl_cmd_identify_get_label_size;
> > >  	cxl_cmd_new_get_health_info;
> > >  	cxl_cmd_health_info_get_maintenance_needed;
> > > diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> > > index 46f99fb..f19ed4f 100644
> > > --- a/cxl/libcxl.h
> > > +++ b/cxl/libcxl.h
> > > @@ -71,7 +71,7 @@ int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev, int fw_len);
> > >  unsigned long long cxl_cmd_identify_get_total_bytes(struct cxl_cmd *cmd);
> > >  unsigned long long cxl_cmd_identify_get_volatile_only_bytes(struct cxl_cmd *cmd);
> > >  unsigned long long cxl_cmd_identify_get_persistent_only_bytes(struct cxl_cmd *cmd);
> > > -unsigned long long cxl_cmd_identify_get_partition_align(struct cxl_cmd *cmd);
> > > +unsigned long long cxl_cmd_identify_get_partition_align_bytes(struct cxl_cmd *cmd);
> > >  unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd);
> > >  struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
> > >  int cxl_cmd_health_info_get_maintenance_needed(struct cxl_cmd *cmd);
> > > -- 
> > > 2.31.1
> > > 
> > 
>
diff mbox series

Patch

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 1fd584a..823bcb0 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1078,7 +1078,7 @@  CXL_EXPORT int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev,
 	return 0;
 }
 
-CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
+CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align_bytes(
 		struct cxl_cmd *cmd)
 {
 	struct cxl_cmd_identify *id =
@@ -1089,7 +1089,7 @@  CXL_EXPORT unsigned long long cxl_cmd_identify_get_partition_align(
 	if (cmd->status < 0)
 		return cmd->status;
 
-	return le64_to_cpu(id->partition_align);
+	return le64_to_cpu(id->partition_align) * CXL_CAPACITY_MULTIPLIER;
 }
 
 CXL_EXPORT unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd)
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index b7e969f..1e2cf05 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -34,7 +34,7 @@  global:
 	cxl_cmd_identify_get_total_bytes;
 	cxl_cmd_identify_get_volatile_only_bytes;
 	cxl_cmd_identify_get_persistent_only_bytes;
-	cxl_cmd_identify_get_partition_align;
+	cxl_cmd_identify_get_partition_align_bytes;
 	cxl_cmd_identify_get_label_size;
 	cxl_cmd_new_get_health_info;
 	cxl_cmd_health_info_get_maintenance_needed;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 46f99fb..f19ed4f 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -71,7 +71,7 @@  int cxl_cmd_identify_get_fw_rev(struct cxl_cmd *cmd, char *fw_rev, int fw_len);
 unsigned long long cxl_cmd_identify_get_total_bytes(struct cxl_cmd *cmd);
 unsigned long long cxl_cmd_identify_get_volatile_only_bytes(struct cxl_cmd *cmd);
 unsigned long long cxl_cmd_identify_get_persistent_only_bytes(struct cxl_cmd *cmd);
-unsigned long long cxl_cmd_identify_get_partition_align(struct cxl_cmd *cmd);
+unsigned long long cxl_cmd_identify_get_partition_align_bytes(struct cxl_cmd *cmd);
 unsigned int cxl_cmd_identify_get_label_size(struct cxl_cmd *cmd);
 struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
 int cxl_cmd_health_info_get_maintenance_needed(struct cxl_cmd *cmd);