[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZtoGzNhrCOPvrnGV@leg>
Date: Thu, 5 Sep 2024 12:30:20 -0700
From: Fan Ni <nifan.cxl@...il.com>
To: ira.weiny@...el.com
Cc: Dave Jiang <dave.jiang@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Navneet Singh <navneet.singh@...el.com>, Chris Mason <clm@...com>,
Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>,
Petr Mladek <pmladek@...e.com>,
Steven Rostedt <rostedt@...dmis.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Jonathan Corbet <corbet@....net>,
Andrew Morton <akpm@...ux-foundation.org>,
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-cxl@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
nvdimm@...ts.linux.dev
Subject: Re: [PATCH v3 18/25] cxl/extent: Process DCD events and realize
region extents
On Fri, Aug 16, 2024 at 09:44:26AM -0500, ira.weiny@...el.com wrote:
> From: Navneet Singh <navneet.singh@...el.com>
>
> A dynamic capacity device (DCD) sends events to signal the host for
> changes in the availability of Dynamic Capacity (DC) memory. These
> events contain extents describing a DPA range and meta data for memory
> to be added or removed. Events may be sent from the device at any time.
>
> Three types of events can be signaled, Add, Release, and Force Release.
>
> On add, the host may accept or reject the memory being offered. If no
> region exists, or the extent is invalid, the extent should be rejected.
> Add extent events may be grouped by a 'more' bit which indicates those
> extents should be processed as a group.
>
> On remove, the host can delay the response until the host is safely not
> using the memory. If no region exists the release can be sent
> immediately. The host may also release extents (or partial extents) at
> any time. Thus the 'more' bit grouping of release events is of less
> value and can be ignored in favor of sending multiple release capacity
> responses for groups of release events.
>
> Force removal is intended as a mechanism between the FM and the device
> and intended only when the host is unresponsive, out of sync, or
> otherwise broken. Purposely ignore force removal events.
>
> Regions are made up of one or more devices which may be surfacing memory
> to the host. Once all devices in a region have surfaced an extent the
> region can expose a corresponding extent for the user to consume.
> Without interleaving a device extent forms a 1:1 relationship with the
> region extent. Immediately surface a region extent upon getting a
> device extent.
>
> Per the specification the device is allowed to offer or remove extents
> at any time. However, anticipated use cases can expect extents to be
> offered, accepted, and removed in well defined chunks.
>
> Simplify extent tracking with the following restrictions.
>
> 1) Flag for removal any extent which overlaps a requested
> release range.
> 2) Refuse the offer of extents which overlap already accepted
> memory ranges.
> 3) Accept again a range which has already been accepted by the
> host. (It is likely the device has an error because it
> should already know that this range was accepted. But from
> the host point of view it is safe to acknowledge that
> acceptance again.)
>
> Management of the region extent devices must be synchronized with
> potential uses of the memory within the DAX layer. Create region extent
> devices as children of the cxl_dax_region device such that the DAX
> region driver can co-drive them and synchronize with the DAX layer.
> Synchronization and management is handled in a subsequent patch.
>
> Process DCD events and create region devices.
>
> Signed-off-by: Navneet Singh <navneet.singh@...el.com>
> Co-developed-by: Ira Weiny <ira.weiny@...el.com>
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>
One more minor inline.
> +static int cxl_validate_extent(struct cxl_memdev_state *mds,
> + struct cxl_extent *extent)
> +{
> + u64 start = le64_to_cpu(extent->start_dpa);
> + u64 length = le64_to_cpu(extent->length);
> + struct device *dev = mds->cxlds.dev;
> +
> + struct range ext_range = (struct range){
> + .start = start,
> + .end = start + length - 1,
> + };
> +
> + if (le16_to_cpu(extent->shared_extn_seq) != 0) {
> + dev_err_ratelimited(dev,
> + "DC extent DPA %par (%*phC) can not be shared\n",
> + &ext_range.start, CXL_EXTENT_TAG_LEN,
> + extent->tag);
> + return -ENXIO;
> + }
> +
> + /* Extents must not cross DC region boundary's */
> + for (int i = 0; i < mds->nr_dc_region; i++) {
> + struct cxl_dc_region_info *dcr = &mds->dc_region[i];
> + struct range region_range = (struct range) {
> + .start = dcr->base,
> + .end = dcr->base + dcr->decode_len - 1,
> + };
> +
> + if (range_contains(®ion_range, &ext_range)) {
> + dev_dbg(dev, "DC extent DPA %par (DCR:%d:%#llx)(%*phC)\n",
> + &ext_range, i, start - dcr->base,
> + CXL_EXTENT_TAG_LEN, extent->tag);
> + return 0;
> + }
> + }
For extent validation, we may need to ensure its size is not 0 based on the spec.
Noted that during testing, do not see issue for the case as 0-sized
extents will be rejected when trying to add even though it passes the
validation.
Fan
Powered by blists - more mailing lists