[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230829172048.000006fb@Huawei.com>
Date: Tue, 29 Aug 2023 17:20:48 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Ira Weiny <ira.weiny@...el.com>
CC: Dan Williams <dan.j.williams@...el.com>,
Navneet Singh <navneet.singh@...el.com>,
Fan Ni <fan.ni@...sung.com>,
Davidlohr Bueso <dave@...olabs.net>,
Dave Jiang <dave.jiang@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC v2 11/18] cxl/region: Expose DC extents on region
driver load
On Mon, 28 Aug 2023 22:21:02 -0700
Ira Weiny <ira.weiny@...el.com> wrote:
> Ultimately user space must associate Dynamic Capacity (DC) extents with
> DAX devices. Remember also that DCD extents may have been accepted
> previous to regions being created and must have references held until
> all higher level regions and DAX devices are done with the memory.
>
> On CXL region driver load scan existing device extents and create CXL
> DAX region extents as needed.
>
> Create abstractions for the extents to be used in DAX region. This
> includes a generic interface to take proper references on the lower
> level CXL region extents.
>
> Also maintain separate objects for the DAX region extent device vs the
> DAX region extent. The DAX region extent device has a shorter life span
> which corresponds to the removal of an extent while a DAX device is
> still using it. In this case an extent continues to exist whilst the
> ability to create new DAX devices on that extent is prevented.
>
> NOTE: Without interleaving; the device, CXL region, and DAX region
> extents have a 1:1:1 relationship. Future support for interleaving will
> maintain a 1:N relationship between CXL region extents and the hardware
> extents.
>
> While the ability to create DAX devices on an extent exists; expose the
> necessary details of DAX region extents by creating a device with the
> following sysfs entries.
>
> /sys/bus/cxl/devices/dax_regionX/extentY
> /sys/bus/cxl/devices/dax_regionX/extentY/length
> /sys/bus/cxl/devices/dax_regionX/extentY/label
>
> Label is a rough analogy to the DC extent tag. As such the DC extent
> tag is used to initially populate the label. However, the label is made
> writeable so that it can be adjusted in the future when forming a DAX
> device.
>
> Signed-off-by: Navneet Singh <navneet.singh@...el.com>
> Co-developed-by: Navneet Singh <navneet.singh@...el.com>
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>
Trivial stuff inline.
> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index 27cf2daaaa79..4dab52496c3f 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -5,6 +5,7 @@
> #ifndef __DAX_PRIVATE_H__
> #define __DAX_PRIVATE_H__
>
> +#include <linux/pgtable.h>
> #include <linux/device.h>
> #include <linux/cdev.h>
> #include <linux/idr.h>
> @@ -40,6 +41,58 @@ struct dax_region {
> struct device *youngest;
> };
>
> +/*
/**
as it's valid kernel doc so no disadvantage really.
> + * struct dax_region_extent - extent data defined by the low level region
> + * driver.
> + * @private_data: lower level region driver data
> + * @ref: track number of dax devices which are using this extent
> + * @get: get reference to low level data
> + * @put: put reference to low level data
I'd like to understand when these are optional - perhaps comment on that?
> + */
> +struct dax_region_extent {
> + void *private_data;
> + struct kref ref;
> + void (*get)(struct dax_region_extent *dr_extent);
> + void (*put)(struct dax_region_extent *dr_extent);
> +};
> +
> +static inline void dr_extent_get(struct dax_region_extent *dr_extent)
> +{
> + if (dr_extent->get)
> + dr_extent->get(dr_extent);
> +}
> +
> +static inline void dr_extent_put(struct dax_region_extent *dr_extent)
> +{
> + if (dr_extent->put)
> + dr_extent->put(dr_extent);
> +}
> +
> +#define DAX_EXTENT_LABEL_LEN 64
blank line here.
> +/**
> + * struct dax_reg_ext_dev - Device object to expose extent information
> + * @dev: device representing this extent
> + * @dr_extent: reference back to private extent data
> + * @offset: offset of this extent
> + * @length: size of this extent
> + * @label: identifier to group extents
> + */
> +struct dax_reg_ext_dev {
> + struct device dev;
> + struct dax_region_extent *dr_extent;
> + resource_size_t offset;
> + resource_size_t length;
> + char label[DAX_EXTENT_LABEL_LEN];
> +};
Powered by blists - more mailing lists