| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <13118eaa-6fae-7b45-9f3e-ab95e7d34729@oracle.com>
Date: Tue, 13 Jun 2023 10:24:30 -0500
From: Eric DeVolder <eric.devolder@...cle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org, david@...hat.com, osalvador@...e.de,
corbet@....net, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, bhe@...hat.com,
ebiederm@...ssion.com, kexec@...ts.infradead.org, hpa@...or.com,
rafael@...nel.org, vgoyal@...hat.com, dyoung@...hat.com,
lf32.dev@...il.com, akpm@...ux-foundation.org,
naveen.n.rao@...ux.vnet.ibm.com, zohar@...ux.ibm.com,
bhelgaas@...gle.com, vbabka@...e.cz, tiwai@...e.de,
seanjc@...gle.com, linux@...ssschuh.net, vschneid@...hat.com,
linux-mm@...ck.org, linux-doc@...r.kernel.org,
sourabhjain@...ux.ibm.com, konrad.wilk@...cle.com,
boris.ostrovsky@...cle.com
Subject: Re: [PATCH v23 4/8] crash: memory and CPU hotplug sysfs attributes
On 6/13/23 03:03, Greg KH wrote:
> On Mon, Jun 12, 2023 at 05:07:08PM -0400, Eric DeVolder wrote:
>> Introduce the crash_hotplug attribute for memory and CPUs for
>> use by userspace. These attributes directly facilitate the udev
>> rule for managing userspace re-loading of the crash kernel upon
>> hot un/plug changes.
>>
>> For memory, expose the crash_hotplug attribute to the
>> /sys/devices/system/memory directory. For example:
>>
>> # udevadm info --attribute-walk /sys/devices/system/memory/memory81
>> looking at device '/devices/system/memory/memory81':
>> KERNEL=="memory81"
>> SUBSYSTEM=="memory"
>> DRIVER==""
>> ATTR{online}=="1"
>> ATTR{phys_device}=="0"
>> ATTR{phys_index}=="00000051"
>> ATTR{removable}=="1"
>> ATTR{state}=="online"
>> ATTR{valid_zones}=="Movable"
>>
>> looking at parent device '/devices/system/memory':
>> KERNELS=="memory"
>> SUBSYSTEMS==""
>> DRIVERS==""
>> ATTRS{auto_online_blocks}=="offline"
>> ATTRS{block_size_bytes}=="8000000"
>> ATTRS{crash_hotplug}=="1"
>>
>> For CPUs, expose the crash_hotplug attribute to the
>> /sys/devices/system/cpu directory. For example:
>>
>> # udevadm info --attribute-walk /sys/devices/system/cpu/cpu0
>> looking at device '/devices/system/cpu/cpu0':
>> KERNEL=="cpu0"
>> SUBSYSTEM=="cpu"
>> DRIVER=="processor"
>> ATTR{crash_notes}=="277c38600"
>> ATTR{crash_notes_size}=="368"
>> ATTR{online}=="1"
>>
>> looking at parent device '/devices/system/cpu':
>> KERNELS=="cpu"
>> SUBSYSTEMS==""
>> DRIVERS==""
>> ATTRS{crash_hotplug}=="1"
>> ATTRS{isolated}==""
>> ATTRS{kernel_max}=="8191"
>> ATTRS{nohz_full}==" (null)"
>> ATTRS{offline}=="4-7"
>> ATTRS{online}=="0-3"
>> ATTRS{possible}=="0-7"
>> ATTRS{present}=="0-3"
>>
>> With these sysfs attributes in place, it is possible to efficiently
>> instruct the udev rule to skip crash kernel reloading for kernels
>> configured with crash hotplug support.
>>
>> For example, the following is the proposed udev rule change for RHEL
>> system 98-kexec.rules (as the first lines of the rule file):
>>
>> # The kernel updates the crash elfcorehdr for CPU and memory changes
>> SUBSYSTEM=="cpu", ATTRS{crash_hotplug}=="1", GOTO="kdump_reload_end"
>> SUBSYSTEM=="memory", ATTRS{crash_hotplug}=="1", GOTO="kdump_reload_end"
>>
>> When examined in the context of 98-kexec.rules, the above rules
>> test if crash_hotplug is set, and if so, the userspace initiated
>> unload-then-reload of the crash kernel is skipped.
>>
>> CPU and memory checks are separated in accordance with
>> CONFIG_HOTPLUG_CPU and CONFIG_MEMORY_HOTPLUG kernel config options.
>> If an architecture supports, for example, memory hotplug but not
>> CPU hotplug, then the /sys/devices/system/memory/crash_hotplug
>> attribute file is present, but the /sys/devices/system/cpu/crash_hotplug
>> attribute file will NOT be present. Thus the udev rule skips
>> userspace processing of memory hot un/plug events, but the udev
>> rule will evaluate false for CPU events, thus allowing userspace to
>> process CPU hot un/plug events (ie the unload-then-reload of the kdump
>> capture kernel).
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@...cle.com>
>> Reviewed-by: Sourabh Jain <sourabhjain@...ux.ibm.com>
>> Acked-by: Hari Bathini <hbathini@...ux.ibm.com>
>> Acked-by: Baoquan He <bhe@...hat.com>
>> ---
>> .../admin-guide/mm/memory-hotplug.rst | 8 ++++++++
>> Documentation/core-api/cpu_hotplug.rst | 18 ++++++++++++++++++
>> drivers/base/cpu.c | 14 ++++++++++++++
>> drivers/base/memory.c | 13 +++++++++++++
>> include/linux/kexec.h | 8 ++++++++
>> 5 files changed, 61 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/mm/memory-hotplug.rst b/Documentation/admin-guide/mm/memory-hotplug.rst
>> index 1b02fe5807cc..eb99d79223a3 100644
>> --- a/Documentation/admin-guide/mm/memory-hotplug.rst
>> +++ b/Documentation/admin-guide/mm/memory-hotplug.rst
>> @@ -291,6 +291,14 @@ The following files are currently defined:
>> Availability depends on the CONFIG_ARCH_MEMORY_PROBE
>> kernel configuration option.
>> ``uevent`` read-write: generic udev file for device subsystems.
>> +``crash_hotplug`` read-only: when changes to the system memory map
>> + occur due to hot un/plug of memory, this file contains
>> + '1' if the kernel updates the kdump capture kernel memory
>> + map itself (via elfcorehdr), or '0' if userspace must update
>> + the kdump capture kernel memory map.
>> +
>> + Availability depends on the CONFIG_MEMORY_HOTPLUG kernel
>> + configuration option.
>> ====================== =========================================================
>>
>> .. note::
>> diff --git a/Documentation/core-api/cpu_hotplug.rst b/Documentation/core-api/cpu_hotplug.rst
>> index f75778d37488..0c8dc3fe5f94 100644
>> --- a/Documentation/core-api/cpu_hotplug.rst
>> +++ b/Documentation/core-api/cpu_hotplug.rst
>> @@ -750,6 +750,24 @@ will receive all events. A script like::
>>
>> can process the event further.
>>
>> +When changes to the CPUs in the system occur, the sysfs file
>> +/sys/devices/system/cpu/crash_hotplug contains '1' if the kernel
>> +updates the kdump capture kernel list of CPUs itself (via elfcorehdr),
>> +or '0' if userspace must update the kdump capture kernel list of CPUs.
>> +
>> +The availability depends on the CONFIG_HOTPLUG_CPU kernel configuration
>> +option.
>> +
>> +To skip userspace processing of CPU hot un/plug events for kdump
>> +(ie the unload-then-reload to obtain a current list of CPUs), this sysfs
>> +file can be used in a udev rule as follows:
>> +
>> + SUBSYSTEM=="cpu", ATTRS{crash_hotplug}=="1", GOTO="kdump_reload_end"
>> +
>> +For a cpu hot un/plug event, if the architecture supports kernel updates
>> +of the elfcorehdr (which contains the list of CPUs), then the rule skips
>> +the unload-then-reload of the kdump capture kernel.
>> +
>> Kernel Inline Documentations Reference
>> ======================================
>>
>> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
>> index c1815b9dae68..06a0c22b37b8 100644
>> --- a/drivers/base/cpu.c
>> +++ b/drivers/base/cpu.c
>> @@ -282,6 +282,17 @@ static ssize_t print_cpus_nohz_full(struct device *dev,
>> static DEVICE_ATTR(nohz_full, 0444, print_cpus_nohz_full, NULL);
>> #endif
>>
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +#include <linux/kexec.h>
>> +static ssize_t crash_hotplug_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return sprintf(buf, "%d\n", crash_hotplug_cpu_support());
>> +}
>> +static DEVICE_ATTR_ADMIN_RO(crash_hotplug);
>> +#endif
>> +
>> static void cpu_device_release(struct device *dev)
>> {
>> /*
>> @@ -469,6 +480,9 @@ static struct attribute *cpu_root_attrs[] = {
>> #ifdef CONFIG_NO_HZ_FULL
>> &dev_attr_nohz_full.attr,
>> #endif
>> +#ifdef CONFIG_HOTPLUG_CPU
>> + &dev_attr_crash_hotplug.attr,
>> +#endif
>> #ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>> &dev_attr_modalias.attr,
>> #endif
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index b456ac213610..24b8ef4c830c 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -490,6 +490,16 @@ static ssize_t auto_online_blocks_store(struct device *dev,
>>
>> static DEVICE_ATTR_RW(auto_online_blocks);
>>
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> +#include <linux/kexec.h>
>> +static ssize_t crash_hotplug_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + return sprintf(buf, "%d\n", crash_hotplug_memory_support());
>> +}
>
> This sysfs file has to be documented in Documentation/ABI/ right?
I will add these entries.
>
> And did you use checkpatch? It should have told you to use sysfs_emit()
> instead...
I did use checkpatch, and it did not reveal that.
[root@...alhost linux]# ./scripts/checkpatch.pl --patch
../v23/0004-crash-memory-and-CPU-hotplug-sysfs-attributes.patch
total: 0 errors, 0 warnings, 103 lines checked
../v23/0004-crash-memory-and-CPU-hotplug-sysfs-attributes.patch has no obvious style problems and is
ready for submission.
This is not the first time it's been suggested to me that checkpatch should have caught a problem;
is there a different invocation than what I'm doing?
>
>> +static DEVICE_ATTR_RO(crash_hotplug);
>> +#endif
>
> All of these #ifdefs should all be removed and instead use the
> is_visible() callback to determine if the attribute is shown or not,
> using the IS_ENABLED() test in the function.
ok, I'll correct this.
Thank you for looking at this!
eric
>
> thanks,
>
> greg k-h
Powered by blists - more mailing lists