Messages in this thread | | | Date | Tue, 5 Sep 2023 20:36:47 -0700 | From | Ira Weiny <> | Subject | Re: [PATCH RFC v2 11/18] cxl/region: Expose DC extents on region driver load |
| |
Jonathan Cameron wrote: > On Mon, 28 Aug 2023 22:21:02 -0700 > Ira Weiny <ira.weiny@intel.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@intel.com> > > Co-developed-by: Navneet Singh <navneet.singh@intel.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.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.
Sure. Done.
> > > + * 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?
They are not optional in this implementation. I got a bit carried away in extrapolating the dax_region away from the lower levels in thinking that some other implementation may not need these.
I will still keep the helpers below though.
> > > + */ > > +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.
Sure. Done
Ira
| |