diff mbox series

[ndctl,3/7] libcxl: apply CXL_CAPACITY_MULTIPLIER to partition alignment field

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

Commit Message

Alison Schofield Jan. 3, 2022, 8:16 p.m. UTC
From: Alison Schofield <alison.schofield@intel.com>

The IDENTIFY mailbox command returns the partition alignment field
expressed in multiples of 256 MB. Use CXL_CAPACITY_MULTIPLIER when
returning this field.

This is in sync with all the other partitioning related fields using
the multiplier.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/lib/libcxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ira Weiny Jan. 6, 2022, 8:40 p.m. UTC | #1
On Mon, Jan 03, 2022 at 12:16:14PM -0800, Schofield, Alison wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The IDENTIFY mailbox command returns the partition alignment field
> expressed in multiples of 256 MB.

Interesting...

I don't think anyone is using this function just yet but this does technically
change the behavior of this function.

Will that break anyone or cxl-cli?

> Use CXL_CAPACITY_MULTIPLIER when
> returning this field.
> 
> This is in sync with all the other partitioning related fields using
> the multiplier.

To me the fact that this was not in bytes implies that the original API of
libcxl was intended to not convert these values.

Vishal may have an opinion.  Should these be in bytes or 'CXL Capacity' units
(ie 256MB's)?

I think I prefer bytes as well.

Ira

> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  cxl/lib/libcxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 715d8e4..85a6c0e 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1086,7 +1086,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)
> -- 
> 2.31.1
>
Verma, Vishal L Jan. 7, 2022, 8:01 p.m. UTC | #2
On Thu, 2022-01-06 at 12:40 -0800, Ira Weiny wrote:
> On Mon, Jan 03, 2022 at 12:16:14PM -0800, Schofield, Alison wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > The IDENTIFY mailbox command returns the partition alignment field
> > expressed in multiples of 256 MB.
> 
> Interesting...
> 
> I don't think anyone is using this function just yet but this does technically
> change the behavior of this function.
> 
> Will that break anyone or cxl-cli?
> 
> > Use CXL_CAPACITY_MULTIPLIER when
> > returning this field.
> > 
> > This is in sync with all the other partitioning related fields using
> > the multiplier.
> 
> To me the fact that this was not in bytes implies that the original API of
> libcxl was intended to not convert these values.
> 
> Vishal may have an opinion.  Should these be in bytes or 'CXL Capacity' units
> (ie 256MB's)?
> 
> I think I prefer bytes as well.

Hm yeah there's not an internal (i.e. cxl-cli) user yet. I think bytes
makes sense. It is common for libcxl to convert spec-isms to whatever
makes sense at the library/cxl-cli level, and for that bytes makes
sense.

> 
> Ira
> 
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  cxl/lib/libcxl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index 715d8e4..85a6c0e 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -1086,7 +1086,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)
> > -- 
> > 2.31.1
> >
diff mbox series

Patch

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 715d8e4..85a6c0e 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1086,7 +1086,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)