On Mon, Feb 7, 2022 at 3:06 PM <[email protected]> wrote:
>
> From: Alison Schofield <[email protected]>
>
> Users may want to view and change partition layouts of CXL memory
> devices that support partitioning. Provide userspace access to
> the device partitioning capabilities as defined in the CXL 2.0
> specification.
This is minor feedback if these end up being re-spun, but "Users may
want..." is too passive for what this is, which is a critical building
block in the provisioning model for PMEM over CXL. So, consider
rewriting in active voice, and avoid underselling the importance of
this capability.
> The first 4 patches add accessors for all the information needed
> to formulate a new partition layout. This info is accessible via
> the libcxl API and a new option in the cxl list command:
>
> "partition_info":{
> "active_volatile_bytes":273535729664,
> "active_persistent_bytes":0,
> "next_volatile_bytes":268435456,
> "next_persistent_bytes":273267294208,
> "total_bytes":273535729664,
> "volatile_only_bytes":0,
> "persistent_only_bytes":0,
> "partition_alignment_bytes":268435456
> }
Is this stale? I.e. we discussed aligning the names to other
'size'-like values in 'ndctl list' and 'cxl list'.
>
> Patch 5 introduces the libcxl interfaces for the SET_PARTITION_INFO
> mailbox command and Patch 6 adds the new CXL command:
>
> Synopsis:
> cxl set-partition <mem0> [<mem1>..<memN>] [<options>]
>
> -t, --type=<type> 'pmem' or 'volatile' (Default: 'pmem')
> -s, --size=<size> size in bytes (Default: all partitionable capacity)
Spell-check does not like "partitionable"
s/partitionable/available/
> -a, --align allow alignment correction
How about:
"Auto-align --size per device's requirement."
> -v, --verbose turn on debug
>
> The CXL command does not offer the IMMEDIATE mode option defined
s/CXL/'cxl set-parition'/
This is a general problem caused by the tool 'cxl' being the same name
as the specification CXL. When it is ambiguous, go ahead and spell out
'cxl <command>'.
> in the CXL 2.0 spec because the CXL kernel driver does not support
> immediate mode yet. Since userspace could use the libcxl API to
> send a SET_PARTITION_INFO mailbox command with immediate mode
> selected, a kernel patch that rejects such requests is in review
> for the CXL driver.
>
> Changes in v4: (from Dan's review)
> - cxl set-partition command: add type (pmem | volatile),
> add defaults for type and size, and add an align option.
> - Replace macros with return statements with functions.
> - Add cxl_set_partition_set_mode() to the libcxl API.
> - Add API documentation to Documentation/cxl/lib/libcxl.txt.
> - Use log_err/info mechanism.
> - Display memdev JSON info upon completion of set-partition command.
> - Remove unneeded casts when accessing command payloads.
> - Name changes - like drop _info suffix, use _size instead of _bytes.
>
> Changes in v3:
> - Back out the API name change to the partition align accessor.
> The API was already released in v72.
> - Man page: collapse into one .txt file.
> - Man page: add to Documentation/meson.build
>
> Changes in v2:
> - Rebase onto https://github.com/pmem/ndctl.git djbw/meson.
> - Clarify that capacities are reported in bytes. (Ira)
> This led to renaming accessors like *_capacity to *_bytes and
> a spattering of changes in the man pages and commit messages.
> - Add missing cxl_cmd_unref() when creating json objects. (Ira)
> - Change the cxl list option to: -I --partition (Dan)
> - Use OPT_STRING for the --volatile_size parameter (Dan, Vishal)
> - Drop extra _get_ in accessor names. (Vishal)
> - Add an accessor for the SET_PARTITION_INFO mbox cmd flag.
> - Display usage_with_options if size parameter is missing.
> - Drop the OPT64 define patch. Use OPT_STRING instead.
>
> Alison Schofield (6):
> libcxl: add GET_PARTITION_INFO mailbox command and accessors
> libcxl: add accessors for capacity fields of the IDENTIFY command
> libcxl: return the partition alignment field in bytes
> cxl: add memdev partition information to cxl-list
> libcxl: add interfaces for SET_PARTITION_INFO mailbox command
> cxl: add command set-partition
>
> Documentation/cxl/cxl-list.txt | 23 +++
> Documentation/cxl/cxl-set-partition.txt | 60 ++++++++
> Documentation/cxl/lib/libcxl.txt | 5 +
> Documentation/cxl/meson.build | 1 +
> cxl/builtin.h | 1 +
> cxl/cxl.c | 1 +
> cxl/filter.c | 2 +
> cxl/filter.h | 1 +
> cxl/json.c | 113 ++++++++++++++
> cxl/lib/libcxl.c | 123 ++++++++++++++-
> cxl/lib/libcxl.sym | 10 ++
> cxl/lib/private.h | 18 +++
> cxl/libcxl.h | 18 +++
> cxl/list.c | 2 +
> cxl/memdev.c | 196 ++++++++++++++++++++++++
> util/json.h | 1 +
> util/size.h | 1 +
> 17 files changed, 575 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/cxl/cxl-set-partition.txt
>
> --
> 2.31.1
>
