From: Dan Williams <dan.j.williams@intel.com>
To: <shiju.jose@huawei.com>, <linux-cxl@vger.kernel.org>,
<linux-acpi@vger.kernel.org>, <linux-mm@kvack.org>,
<dan.j.williams@intel.com>, <dave@stgolabs.net>,
<jonathan.cameron@huawei.com>, <dave.jiang@intel.com>,
<alison.schofield@intel.com>, <vishal.l.verma@intel.com>,
<ira.weiny@intel.com>
Cc: <linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<david@redhat.com>, <Vilas.Sridharan@amd.com>,
<leo.duran@amd.com>, <Yazen.Ghannam@amd.com>,
<rientjes@google.com>, <jiaqiyan@google.com>,
<tony.luck@intel.com>, <Jon.Grimm@amd.com>,
<dave.hansen@linux.intel.com>, <rafael@kernel.org>,
<lenb@kernel.org>, <naoya.horiguchi@nec.com>,
<james.morse@arm.com>, <jthoughton@google.com>,
<somasundaram.a@hpe.com>, <erdemaktas@google.com>,
<pgonda@google.com>, <duenwen@google.com>,
<mike.malvestuto@intel.com>, <gthelen@google.com>,
<wschwartz@amperecomputing.com>, <dferguson@amperecomputing.com>,
<wbs@os.amperecomputing.com>, <nifan.cxl@gmail.com>,
<tanxiaofei@huawei.com>, <prime.zeng@hisilicon.com>,
<kangkang.shen@futurewei.com>, <wanghuiqiang@huawei.com>,
<linuxarm@huawei.com>, <shiju.jose@huawei.com>
Subject: RE: [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem
Date: Thu, 9 May 2024 14:47:56 -0700 [thread overview]
Message-ID: <663d448c2ef3_1c0a1929453@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20240419164720.1765-2-shiju.jose@huawei.com>
shiju.jose@ wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> 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@huawei.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
> .../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@vger.kernel.org
> +Description:
> + The ras/ class subdirectory belongs to the
> + common ras features such as scrub subsystem.
Why create "ras" class versus just a "srcub" class? I am otherwise not
aware of a precedent for class device hierarchy. For example, on my
system there is:
/sys/class/
├── scsi_device
├── scsi_disk
├── scsi_generic
└── scsi_host
...not:
/sys/class/scsi/
├── device
├── disk
├── generic
└── host
> +
> +What: /sys/class/ras/rasX/scrub/
> +Date: March 2024
> +KernelVersion: 6.9
> +Contact: linux-kernel@vger.kernel.org
> +Description:
> + The /sys/class/ras/ras{0,1,2,3,...}/scrub directories
> + correspond to each scrub device registered with the
> + scrub subsystem.
I notice there are some visibility rules in the code, but those
expectations are not documented here.
This documentation would also help developers writing new users of
scrub_device_register().
> +
> +What: /sys/class/ras/rasX/scrub/name
> +Date: March 2024
> +KernelVersion: 6.9
> +Contact: linux-kernel@vger.kernel.org
> +Description:
> + (RO) name of the memory scrubber
> +
> +What: /sys/class/ras/rasX/scrub/enable_background
> +Date: March 2024
> +KernelVersion: 6.9
> +Contact: linux-kernel@vger.kernel.org
> +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@vger.kernel.org
> +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@vger.kernel.org
> +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;
It strikes me as somewhat pointless to have such a thin sysfs
implementation whose only job is to call down into a callback to do the
work. Unless there are other consumers of 'struct scrub_ops' outside of
these sysfs files why not just have the low-level drivers register their
corresponding attributes themselves?
Unless the functionality is truly generic just let the low-level driver
be responsible for conforming to the sysfs ABI expectations, and, for
example, each register their own "enable_background" attribute if they
support that semantic.
So scrub_device_register() would grow a 'const struct attribute_group
**groups' argument, or something along those lines.
next prev parent reply other threads:[~2024-05-09 21:48 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 16:47 [RFC PATCH v8 00/10] ras: scrub: introduce subsystem + CXL/ACPI-RAS2 drivers shiju.jose
2024-04-19 16:47 ` [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem shiju.jose
2024-04-24 20:25 ` fan
2024-04-25 10:38 ` Shiju Jose
2024-04-25 10:15 ` Borislav Petkov
2024-04-25 18:11 ` Shiju Jose
2024-05-06 10:30 ` Borislav Petkov
2024-05-08 16:59 ` Shiju Jose
2024-05-08 17:20 ` Borislav Petkov
2024-05-08 17:44 ` Shiju Jose
2024-05-08 19:25 ` Borislav Petkov
2024-05-09 9:19 ` Jonathan Cameron
2024-05-09 15:52 ` Borislav Petkov
2024-05-09 20:03 ` Borislav Petkov
2024-05-09 21:21 ` Dan Williams
2024-05-09 21:51 ` Borislav Petkov
2024-05-09 22:59 ` Dan Williams
2024-05-10 9:25 ` Borislav Petkov
2024-05-10 17:13 ` Dan Williams
2024-05-11 10:17 ` Borislav Petkov
2024-05-17 11:15 ` Jonathan Cameron
2024-05-17 11:44 ` Jonathan Cameron
2024-05-21 8:06 ` Borislav Petkov
2024-05-22 9:40 ` Jonathan Cameron
2024-05-27 9:09 ` Borislav Petkov
2024-05-20 10:54 ` Shiju Jose
2024-05-20 11:58 ` Jonathan Cameron
2024-05-27 9:21 ` Borislav Petkov
2024-05-28 9:06 ` Jonathan Cameron
2024-06-06 16:05 ` Borislav Petkov
2024-05-10 13:31 ` Jonathan Cameron
2024-05-09 21:47 ` Dan Williams [this message]
2024-05-10 9:03 ` Jonathan Cameron
2024-04-19 16:47 ` [RFC PATCH v8 02/10] cxl/mbox: Add GET_SUPPORTED_FEATURES mailbox command shiju.jose
2024-04-19 16:47 ` [RFC PATCH v8 03/10] cxl/mbox: Add GET_FEATURE " shiju.jose
2024-04-24 23:19 ` fan
2024-04-25 10:38 ` Shiju Jose
2024-04-19 16:47 ` [RFC PATCH v8 04/10] cxl/mbox: Add SET_FEATURE " shiju.jose
2024-04-25 17:26 ` fan
2024-04-19 16:47 ` [RFC PATCH v8 05/10] cxl/memscrub: Add CXL device patrol scrub control feature shiju.jose
2024-04-26 23:56 ` fan
2024-04-29 11:20 ` Shiju Jose
2024-04-29 12:21 ` Jonathan Cameron
2024-05-10 0:26 ` Dan Williams
2024-05-10 11:23 ` Jonathan Cameron
2024-04-19 16:47 ` [RFC PATCH v8 06/10] ACPICA: Add __free() based cleanup function for acpi_put_table shiju.jose
2024-04-19 18:06 ` Jonathan Cameron
2024-04-19 16:47 ` [RFC PATCH v8 07/10] platform: Add __free() based cleanup function for platform_device_put shiju.jose
2024-04-19 16:47 ` [RFC PATCH v8 08/10] ACPI:RAS2: Add ACPI RAS2 driver shiju.jose
2024-06-05 21:32 ` Daniel Ferguson
2024-04-19 16:47 ` [RFC PATCH v8 09/10] ras: scrub: Add scrub control attributes for ACPI RAS2 shiju.jose
2024-04-19 16:47 ` [RFC PATCH v8 10/10] ras: scrub: ACPI RAS2: Add memory ACPI RAS2 driver shiju.jose
2024-06-05 21:33 ` Daniel Ferguson
2024-06-07 15:46 ` Shiju Jose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=663d448c2ef3_1c0a1929453@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=Jon.Grimm@amd.com \
--cc=Vilas.Sridharan@amd.com \
--cc=Yazen.Ghannam@amd.com \
--cc=alison.schofield@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=david@redhat.com \
--cc=dferguson@amperecomputing.com \
--cc=duenwen@google.com \
--cc=erdemaktas@google.com \
--cc=gthelen@google.com \
--cc=ira.weiny@intel.com \
--cc=james.morse@arm.com \
--cc=jiaqiyan@google.com \
--cc=jonathan.cameron@huawei.com \
--cc=jthoughton@google.com \
--cc=kangkang.shen@futurewei.com \
--cc=lenb@kernel.org \
--cc=leo.duran@amd.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxarm@huawei.com \
--cc=mike.malvestuto@intel.com \
--cc=naoya.horiguchi@nec.com \
--cc=nifan.cxl@gmail.com \
--cc=pgonda@google.com \
--cc=prime.zeng@hisilicon.com \
--cc=rafael@kernel.org \
--cc=rientjes@google.com \
--cc=shiju.jose@huawei.com \
--cc=somasundaram.a@hpe.com \
--cc=tanxiaofei@huawei.com \
--cc=tony.luck@intel.com \
--cc=vishal.l.verma@intel.com \
--cc=wanghuiqiang@huawei.com \
--cc=wbs@os.amperecomputing.com \
--cc=wschwartz@amperecomputing.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).