Dave Jiang wrote: > In order to address the issue with being able to expose qos_class sysfs > attributes under 'ram' and 'pmem' sub-directories, the attributes must > be defined as static attributes rather than under driver->dev_groups. > To avoid implementing locking for accessing the 'struct cxl_dpa_perf` > lists, convert the list to a single 'struct cxl_dpa_perf' entry in > preparation to move the attributes to statically defined. > > Link: > https://lore.kernel.org/linux-cxl/65b200ba228f_2d43c29...@dwillia2-mobl3.amr.corp.intel.com.notmuch/ > Suggested-by: Dan Williams <dan.j.willi...@intel.com> > Signed-off-by: Dave Jiang <dave.ji...@intel.com> > --- > drivers/cxl/core/cdat.c | 90 +++++++++++++---------------------------- > drivers/cxl/core/mbox.c | 4 +- > drivers/cxl/cxlmem.h | 10 ++--- > drivers/cxl/mem.c | 25 ++++-------- > 4 files changed, 42 insertions(+), 87 deletions(-)
Oh, wow, this looks wonderful! I was expecting the lists to still be there, just moved out of 'struct cxl_dev_state'. Am I reading this right, the work to select and validate the "best" performance per partition can be done without list walking? If so, great! [..] > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index c5c9d8e0d88d..a62099e47d71 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -221,16 +221,13 @@ static ssize_t ram_qos_class_show(struct device *dev, > struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > struct cxl_dev_state *cxlds = cxlmd->cxlds; > struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); > - struct cxl_dpa_perf *dpa_perf; > + struct cxl_dpa_perf *dpa_perf = &mds->ram_perf; > > if (!dev->driver) > return -ENOENT; This can be deleted since it is racy being referenced without the device_lock(), and nothing in this routine requires the device to be locked. > > - if (list_empty(&mds->ram_perf_list)) > - return -ENOENT; > - > - dpa_perf = list_first_entry(&mds->ram_perf_list, struct cxl_dpa_perf, > - list); > + if (dpa_perf->qos_class == CXL_QOS_CLASS_INVALID) > + return -ENODATA; As long as is_visible() checks for CXL_QOS_CLASS_INVALID there is no need to add error handling in this _show() routine. > > return sysfs_emit(buf, "%d\n", dpa_perf->qos_class); > } > @@ -244,16 +241,10 @@ static ssize_t pmem_qos_class_show(struct device *dev, > struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > struct cxl_dev_state *cxlds = cxlmd->cxlds; > struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); > - struct cxl_dpa_perf *dpa_perf; > + struct cxl_dpa_perf *dpa_perf = &mds->pmem_perf; > > - if (!dev->driver) > - return -ENOENT; Ah, good, you deleted it this time. > - > - if (list_empty(&mds->pmem_perf_list)) > - return -ENOENT; > - > - dpa_perf = list_first_entry(&mds->pmem_perf_list, struct cxl_dpa_perf, > - list); > + if (dpa_perf->qos_class == CXL_QOS_CLASS_INVALID) > + return -ENODATA; This can go.
