Re: [PATCH v4 6/6] powerpc/fadump: sysfs for fadump memory reservation

Greg KH
2019-12-06T12:49:00Z

On Fri, Dec 06, 2019 at 05:54:34PM +0530, Sourabh Jain wrote:
> Add a sys interface to allow querying the memory reserved by FADump for
> saving the crash dump.
>
> Also added Documentation/ABI for the new sysfs file.
>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
> Documentation/ABI/testing/sysfs-kernel-fadump | 7 +++++++
> Documentation/powerpc/firmware-assisted-dump.rst | 5 +++++
> arch/powerpc/kernel/fadump.c | 15 +++++++++++++++
> 3 files changed, 27 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-kernel-fadump b/Documentation/ABI/testing/sysfs-kernel-fadump
> index 5d988b919e81..8f7a64a81783 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-fadump
> +++ b/Documentation/ABI/testing/sysfs-kernel-fadump
> @@ -31,3 +31,10 @@ Description: write only
> the system is booted to capture the vmcore using FADump.
> It is used to release the memory reserved by FADump to
> save the crash dump.
> +
> +What: /sys/kernel/fadump/mem_reserved
> +Date: Dec 2019
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: read only
> + Provide information about the amount of memory reserved by
> + FADump to save the crash dump in bytes.
> diff --git a/Documentation/powerpc/firmware-assisted-dump.rst b/Documentation/powerpc/firmware-assisted-dump.rst
> index 365c10209ef3..04993eaf3113 100644
> --- a/Documentation/powerpc/firmware-assisted-dump.rst
> +++ b/Documentation/powerpc/firmware-assisted-dump.rst
> @@ -268,6 +268,11 @@ Here is the list of files under kernel sysfs:
> be handled and vmcore will not be captured. This interface can be
> easily integrated with kdump service start/stop.
>
> + /sys/kernel/fadump/mem_reserved
> +
> + This is used to display the memory reserved by FADump for saving the
> + crash dump.
> +
> /sys/kernel/fadump_release_mem
> This file is available only when FADump is active during
> second kernel. This is used to release the reserved memory
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 41a3cda81791..b2af51b7c750 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1357,6 +1357,13 @@ static ssize_t fadump_enabled_show(struct kobject *kobj,
> return sprintf(buf, "%d\n", fw_dump.fadump_enabled);
> }
>
> +static ssize_t fadump_mem_reserved_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%ld\n", fw_dump.reserve_dump_area_size);
> +}
> +
> static ssize_t fadump_register_show(struct kobject *kobj,
> struct kobj_attribute *attr,
> char *buf)
> @@ -1430,6 +1437,10 @@ static struct kobj_attribute enable_attr = __ATTR(enabled,
> static struct kobj_attribute register_attr = __ATTR(registered,
> 0644, fadump_register_show,
> fadump_register_store);
> +static struct kobj_attribute mem_reserved_attr = __ATTR(mem_reserved,
> + 0444, fadump_mem_reserved_show,
> + NULL);

__ATTRI_RO()?

> +
>
> DEFINE_SHOW_ATTRIBUTE(fadump_region);
>
> @@ -1464,6 +1475,10 @@ static void fadump_init_files(void)
> pr_err("unable to create release_mem sysfs file (%d)\n",
> rc);
> }
> + rc = sysfs_create_file(fadump_kobj, &mem_reserved_attr.attr);
> + if (rc)
> + pr_err("unable to create mem_reserved sysfs file (%d)\n",
> + rc);

Again, put it in an attribute group, that would have only required one
line, and not this mess of not cleaning up if something went wrong.

thanks,

greg k-h