linux-kernel - Re: [PATCH 03/26] cxl/mem: Read dynamic capacity configuration from the device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 5 Apr 2024 11:09:20 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Jørgen Hansen <Jorgen.Hansen@....com>,
	"ira.weiny@...el.com" <ira.weiny@...el.com>, Dave Jiang
	<dave.jiang@...el.com>, Fan Ni <fan.ni@...sung.com>, Jonathan Cameron
	<Jonathan.Cameron@...wei.com>, Navneet Singh <navneet.singh@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>, Davidlohr Bueso
	<dave@...olabs.net>, Alison Schofield <alison.schofield@...el.com>, "Vishal
 Verma" <vishal.l.verma@...el.com>, "linux-btrfs@...r.kernel.org"
	<linux-btrfs@...r.kernel.org>, "linux-cxl@...r.kernel.org"
	<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 03/26] cxl/mem: Read dynamic capacity configuration from
 the device

Jørgen Hansen wrote:
> On 3/25/24 00:18, ira.weiny@...el.com wrote:
> 
> > From: Navneet Singh <navneet.singh@...el.com>
> > 

[snip]

> >   /**
> >    * struct cxl_memdev_state - Generic Type-3 Memory Device Class driver data
> >    *
> > @@ -467,6 +482,8 @@ struct cxl_dev_state {
> >    * @enabled_cmds: Hardware commands found enabled in CEL.
> >    * @exclusive_cmds: Commands that are kernel-internal only
> >    * @total_bytes: sum of all possible capacities
> > + * @static_cap: Sum of static RAM and PMEM capacities
> > + * @dynamic_cap: Complete DPA range occupied by DC regions
> 
> How about naming these total_range, static_cap and dynamic_range to make 
> it clear that the DPA range occupied by DC regions isn't necessarily 
> usable capacity (as opposed to the static_cap where the spec defines it 
> as usable capacity).

I thought this was a good idea but on second thought these are not range
variables at all.  They really represent the various lengths of the
resources.

For total_bytes the documentation already says 'sum of all __possible__
capacities'.

I think you have a point for the new fields though.  They should all be
named in some consistent manner and documented as such.

So I propose:

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 94531af018f8..9c18b229f69a 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -481,9 +481,9 @@ struct cxl_dc_region_info {
  * @dcd_cmds: List of DCD commands implemented by memory device
  * @enabled_cmds: Hardware commands found enabled in CEL.
  * @exclusive_cmds: Commands that are kernel-internal only
- * @total_bytes: sum of all possible capacities
- * @static_cap: Sum of static RAM and PMEM capacities
- * @dynamic_cap: Complete DPA range occupied by DC regions
+ * @total_bytes: length of all possible capacities
+ * @static_bytes: length of possible static RAM and PMEM partitions
+ * @dynamic_bytes: length of possible DC partitions (DC Regions)
  * @volatile_only_bytes: hard volatile capacity
  * @persistent_only_bytes: hard persistent capacity
  * @partition_align_bytes: alignment size for partition-able capacity
@@ -515,8 +515,8 @@ struct cxl_memdev_state {
        DECLARE_BITMAP(exclusive_cmds, CXL_MEM_COMMAND_ID_MAX);
 
        u64 total_bytes;
-       u64 static_cap;
-       u64 dynamic_cap;
+       u64 static_bytes;
+       u64 dynamic_bytes;
        u64 volatile_only_bytes;
        u64 persistent_only_bytes;
        u64 partition_align_bytes;