| Message ID | ca1821eee9f8e2372e378165d5c24bbf9161e6fe.1641965853.git.alison.schofield@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add partitioning support for CXL memdevs | expand |
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 >
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 > > >
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 --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);