On Tue, Feb 08, 2022 at 09:23:54AM -0800, Dan Williams wrote:
> 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.
Yes! I have some words you gave me in another commit I will draw upon.
>
> > 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'.
>
Yes - that is STALE. The cxl-list patch commit msg has it right.
Will fix here.
"partition_info":{
"active_volatile_size":273535729664,
"active_persistent_size":0,
"next_volatile_size":0,
"next_persistent_size":0,
"total_size":273535729664,
"volatile_only_size":0,
"persistent_only_size":0,
"partition_alignment_size":268435456
}
> >
> > 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/
hmm... passes my spell check, but alas, it is overuse of the root word.
I like available. Will change.
>
> > -a, --align allow alignment correction
>
> How about:
>
> "Auto-align --size per device's requirement."
>
So much better. Thanks.
> > -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>'.
>
Got it.
snip...
