Re: [RFC PATCH] memory-hotplug: add sysfs immovable_mem attribute

From: Greg KH
Date: Wed Dec 27 2017 - 07:47:27 EST


On Wed, Dec 27, 2017 at 08:30:12PM +0800, Chao Fan wrote:
> In sometimes users specify the memory region in immovable node in
> some kernel commandline, such as "kernel_core" or the "immovable_mem="
> in the patchset that I have send. But users don't know the memory
> region. So add this interface to print it.
>
> It will show like this: "nn@ss,nn@ss,...". "nn" means the size of memory
> region, "ss" means the start position of this region.
>
> Signed-off-by: Chao Fan <fanc.fnst@xxxxxxxxxxxxxx>
> ---
> drivers/base/memory.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)

Why did you not also create the needed Documentation/ABI/ file update?

That's required for sysfs attributes.

>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 1d60b58a8c19..9cadf1a9dccb 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -25,6 +25,7 @@
>
> #include <linux/atomic.h>
> #include <linux/uaccess.h>
> +#include <linux/acpi.h>
>
> static DEFINE_MUTEX(mem_sysfs_mutex);
>
> @@ -389,6 +390,52 @@ static ssize_t show_phys_device(struct device *dev,
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> +/*
> + * Immovable memory region
> + */
> +
> +static ssize_t
> +show_immovable_mem(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct acpi_table_header *table_header = NULL;
> + struct acpi_srat_mem_affinity *ma;
> + struct acpi_subtable_header *th;
> + unsigned long long table_size;
> + unsigned long long table_end;
> + char pbuf[35], *p = buf;
> + int len;
> +
> + acpi_get_table(ACPI_SIG_SRAT, 0, &table_header);
> +
> + table_size = sizeof(struct acpi_table_srat);
> + table_end = (unsigned long)table_header + table_header->length;
> + th = (struct acpi_subtable_header *)((unsigned long)
> + table_header + table_size);
> +
> + while (((unsigned long)th) +
> + sizeof(struct acpi_subtable_header) < table_end) {
> + if (th->type == 1) {
> + ma = (struct acpi_srat_mem_affinity *)th;
> + if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)
> + continue;
> + len = sprintf(pbuf, "%llx@%llx",
> + ma->length, ma->base_address);

sysfs is "one value per file", and if you ever have to care if you are
overrunning the length of the buffer, that's a huge hint you are doing
something wrong here.

sorry,

greg k-h