RE: [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>-----Original Message-----
>From: fan <nifan.cxl@xxxxxxxxx>
>Sent: 24 April 2024 21:26
>To: Shiju Jose <shiju.jose@xxxxxxxxxx>
>Cc: linux-cxl@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; linux-
>mm@xxxxxxxxx; dan.j.williams@xxxxxxxxx; dave@xxxxxxxxxxxx; Jonathan
>Cameron <jonathan.cameron@xxxxxxxxxx>; dave.jiang@xxxxxxxxx;
>alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx; ira.weiny@xxxxxxxxx;
>linux-edac@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; david@xxxxxxxxxx;
>Vilas.Sridharan@xxxxxxx; leo.duran@xxxxxxx; Yazen.Ghannam@xxxxxxx;
>rientjes@xxxxxxxxxx; jiaqiyan@xxxxxxxxxx; tony.luck@xxxxxxxxx;
>Jon.Grimm@xxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx;
>lenb@xxxxxxxxxx; naoya.horiguchi@xxxxxxx; james.morse@xxxxxxx;
>jthoughton@xxxxxxxxxx; somasundaram.a@xxxxxxx;
>erdemaktas@xxxxxxxxxx; pgonda@xxxxxxxxxx; duenwen@xxxxxxxxxx;
>mike.malvestuto@xxxxxxxxx; gthelen@xxxxxxxxxx;
>wschwartz@xxxxxxxxxxxxxxxxxxx; dferguson@xxxxxxxxxxxxxxxxxxx;
>wbs@xxxxxxxxxxxxxxxxxxxxxx; nifan.cxl@xxxxxxxxx; tanxiaofei
><tanxiaofei@xxxxxxxxxx>; Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>;
>kangkang.shen@xxxxxxxxxxxxx; wanghuiqiang <wanghuiqiang@xxxxxxxxxx>;
>Linuxarm <linuxarm@xxxxxxxxxx>
>Subject: Re: [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem
>
>On Sat, Apr 20, 2024 at 12:47:10AM +0800, shiju.jose@xxxxxxxxxx wrote:
>> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>>
>> Add scrub subsystem supports configuring the memory scrubbers in the
>> system. The scrub subsystem provides the interface for registering the
>> scrub devices. The scrub control attributes are provided to the user
>> in /sys/class/ras/rasX/scrub
>>
>> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
>> ---
>>  .../ABI/testing/sysfs-class-scrub-configure   |  47 +++
>>  drivers/ras/Kconfig                           |   7 +
>>  drivers/ras/Makefile                          |   1 +
>>  drivers/ras/memory_scrub.c                    | 271 ++++++++++++++++++
>>  include/linux/memory_scrub.h                  |  37 +++
>>  5 files changed, 363 insertions(+)
>>  create mode 100644
>> Documentation/ABI/testing/sysfs-class-scrub-configure
>>  create mode 100755 drivers/ras/memory_scrub.c  create mode 100755
>> include/linux/memory_scrub.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure
>> b/Documentation/ABI/testing/sysfs-class-scrub-configure
>> new file mode 100644
>> index 000000000000..3ed77dbb00ad
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
>> @@ -0,0 +1,47 @@
>> +What:		/sys/class/ras/
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		The ras/ class subdirectory belongs to the
>> +		common ras features such as scrub subsystem.
>> +
>> +What:		/sys/class/ras/rasX/scrub/
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		The /sys/class/ras/ras{0,1,2,3,...}/scrub directories
>> +		correspond to each scrub device registered with the
>> +		scrub subsystem.
>> +
>> +What:		/sys/class/ras/rasX/scrub/name
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		(RO) name of the memory scrubber
>> +
>> +What:		/sys/class/ras/rasX/scrub/enable_background
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		(RW) Enable/Disable background(patrol) scrubbing if supported.
>> +
>> +What:		/sys/class/ras/rasX/scrub/rate_available
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		(RO) Supported range for the scrub rate by the scrubber.
>> +		The scrub rate represents in hours.
>> +
>> +What:		/sys/class/ras/rasX/scrub/rate
>> +Date:		March 2024
>> +KernelVersion:	6.9
>> +Contact:	linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> +		(RW) The scrub rate specified and it must be with in the
>> +		supported range by the scrubber.
>> +		The scrub rate represents in hours.
>> diff --git a/drivers/ras/Kconfig b/drivers/ras/Kconfig index
>> fc4f4bb94a4c..181701479564 100644
>> --- a/drivers/ras/Kconfig
>> +++ b/drivers/ras/Kconfig
>> @@ -46,4 +46,11 @@ config RAS_FMPM
>>  	  Memory will be retired during boot time and run time depending on
>>  	  platform-specific policies.
>>
>> +config SCRUB
>> +	tristate "Memory scrub driver"
>> +	help
>> +	  This option selects the memory scrub subsystem, supports
>> +	  configuring the parameters of underlying scrubbers in the
>> +	  system for the DRAM memories.
>> +
>>  endif
>> diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile index
>> 11f95d59d397..89bcf0d84355 100644
>> --- a/drivers/ras/Makefile
>> +++ b/drivers/ras/Makefile
>> @@ -2,6 +2,7 @@
>>  obj-$(CONFIG_RAS)	+= ras.o
>>  obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
>>  obj-$(CONFIG_RAS_CEC)	+= cec.o
>> +obj-$(CONFIG_SCRUB)	+= memory_scrub.o
>>
>>  obj-$(CONFIG_RAS_FMPM)	+= amd/fmpm.o
>>  obj-y			+= amd/atl/
>> diff --git a/drivers/ras/memory_scrub.c b/drivers/ras/memory_scrub.c
>> new file mode 100755 index 000000000000..7e995380ec3a
>> --- /dev/null
>> +++ b/drivers/ras/memory_scrub.c
>> @@ -0,0 +1,271 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Memory scrub subsystem supports configuring the registered
>> + * memory scrubbers.
>> + *
>> + * Copyright (c) 2024 HiSilicon Limited.
>> + */
>> +
>> +#define pr_fmt(fmt)     "MEM SCRUB: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/kfifo.h>
>> +#include <linux/memory_scrub.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +
>> +/* memory scrubber config definitions */ #define SCRUB_ID_PREFIX
>> +"ras"
>> +#define SCRUB_ID_FORMAT SCRUB_ID_PREFIX "%d"
>> +
>> +static DEFINE_IDA(scrub_ida);
>> +
>> +struct scrub_device {
>> +	int id;
>> +	struct device dev;
>> +	const struct scrub_ops *ops;
>> +};
>> +
>> +#define to_scrub_device(d) container_of(d, struct scrub_device, dev)
>> +static ssize_t enable_background_store(struct device *dev,
>> +				       struct device_attribute *attr,
>> +				       const char *buf, size_t len) {
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	bool enable;
>> +	int ret;
>> +
>> +	ret = kstrtobool(buf, &enable);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = scrub_dev->ops->set_enabled_bg(dev, enable);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t enable_background_show(struct device *dev,
>> +				      struct device_attribute *attr, char *buf) {
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	bool enable;
>> +	int ret;
>> +
>> +	ret = scrub_dev->ops->get_enabled_bg(dev, &enable);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "%d\n", enable); }
>> +
>> +static ssize_t name_show(struct device *dev,
>> +			 struct device_attribute *attr, char *buf) {
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	int ret;
>> +
>> +	ret = scrub_dev->ops->get_name(dev, buf);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return strlen(buf);
>> +}
>> +
>> +static ssize_t rate_show(struct device *dev, struct device_attribute *attr,
>> +			 char *buf)
>> +{
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	u64 val;
>> +	int ret;
>> +
>> +	ret = scrub_dev->ops->rate_read(dev, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "0x%llx\n", val); }
>> +
>> +static ssize_t rate_store(struct device *dev, struct device_attribute *attr,
>> +			  const char *buf, size_t len)
>> +{
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	long val;
>> +	int ret;
>> +
>> +	ret = kstrtol(buf, 10, &val);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = scrub_dev->ops->rate_write(dev, val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t rate_available_show(struct device *dev,
>> +				   struct device_attribute *attr,
>> +				   char *buf)
>> +{
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	u64 min_sr, max_sr;
>> +	int ret;
>> +
>> +	ret = scrub_dev->ops->rate_avail_range(dev, &min_sr, &max_sr);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "0x%llx-0x%llx\n", min_sr, max_sr); }
>> +
>> +DEVICE_ATTR_RW(enable_background);
>> +DEVICE_ATTR_RO(name);
>> +DEVICE_ATTR_RW(rate);
>> +DEVICE_ATTR_RO(rate_available);
>> +
>> +static struct attribute *scrub_attrs[] = {
>> +	&dev_attr_enable_background.attr,
>> +	&dev_attr_name.attr,
>> +	&dev_attr_rate.attr,
>> +	&dev_attr_rate_available.attr,
>> +	NULL
>> +};
>> +
>> +static umode_t scrub_attr_visible(struct kobject *kobj,
>> +				  struct attribute *a, int attr_id) {
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +	const struct scrub_ops *ops = scrub_dev->ops;
>> +
>> +	if (a == &dev_attr_enable_background.attr) {
>> +		if (ops->set_enabled_bg && ops->get_enabled_bg)
>> +			return a->mode;
>> +		if (ops->get_enabled_bg)
>> +			return 0444;
>> +		return 0;
>> +	}
>> +	if (a == &dev_attr_name.attr)
>> +		return ops->get_name ? a->mode : 0;
>> +	if (a == &dev_attr_rate_available.attr)
>> +		return ops->rate_avail_range ? a->mode : 0;
>> +	if (a == &dev_attr_rate.attr) { /* Write only makes little sense */
>> +		if (ops->rate_read && ops->rate_write)
>> +			return a->mode;
>> +		if (ops->rate_read)
>> +			return 0444;
>> +		return 0;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct attribute_group scrub_attr_group = {
>> +	.name		= "scrub",
>> +	.attrs		= scrub_attrs,
>> +	.is_visible	= scrub_attr_visible,
>> +};
>> +
>> +static const struct attribute_group *scrub_attr_groups[] = {
>> +	&scrub_attr_group,
>> +	NULL
>> +};
>> +
>> +static void scrub_dev_release(struct device *dev) {
>> +	struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +
>> +	ida_free(&scrub_ida, scrub_dev->id);
>> +	kfree(scrub_dev);
>> +}
>> +
>> +static struct class scrub_class = {
>> +	.name = "ras",
>> +	.dev_groups = scrub_attr_groups,
>> +	.dev_release = scrub_dev_release,
>> +};
>> +
>> +static struct device *
>> +scrub_device_register(struct device *parent, void *drvdata,
>> +		      const struct scrub_ops *ops)
>> +{
>> +	struct scrub_device *scrub_dev;
>> +	struct device *hdev;
>> +	int err;
>> +
>> +	scrub_dev = kzalloc(sizeof(*scrub_dev), GFP_KERNEL);
>> +	if (!scrub_dev)
>> +		return ERR_PTR(-ENOMEM);
>> +	hdev = &scrub_dev->dev;
>> +
>> +	scrub_dev->id = ida_alloc(&scrub_ida, GFP_KERNEL);
>> +	if (scrub_dev->id < 0) {
>> +		kfree(scrub_dev);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>> +	scrub_dev->ops = ops;
>> +	hdev->class = &scrub_class;
>> +	hdev->parent = parent;
>> +	dev_set_drvdata(hdev, drvdata);
>> +	dev_set_name(hdev, SCRUB_ID_FORMAT, scrub_dev->id);
>
>Need to check the return value of dev_set_name?
Will do, though checking return value of dev_set_name() is not common in the kernel.

>
>fan
>
>> +	err = device_register(hdev);
>> +	if (err) {

Thanks,
Shiju





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux