2024-01-10 14:02:13

by Sumanth Korikkar

[permalink] [raw]
Subject: [PATCH] mm/memory_hotplug: fix memmap_on_memory sysfs value retrieval

The set_memmap_mode() function stores the kernel parameter memmap mode
as an integer. However, the get_memmap_mode() function utilizes
param_get_bool() to fetch the value as a boolean, leading to potential
endianness issue. On Big-endian architectures, the memmap_on_memory is
consistently displayed as 'N' regardless of its actual status.

To address this endianness problem, the solution involves obtaining the
mode as an integer. This adjustment ensures the proper display of the
memmap_on_memory parameter, presenting it as one of the following
options: Force, Y, or N.

Fixes: 2d1f649c7c08 ("mm/memory_hotplug: support memmap_on_memory when memmap is not aligned to pageblocks")
Suggested-by: Gerald Schaefer <[email protected]>
Cc: <[email protected]> # v6.6+
Signed-off-by: Sumanth Korikkar <[email protected]>
---
mm/memory_hotplug.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b944e8bf1911..707027f69150 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -101,9 +101,11 @@ static int set_memmap_mode(const char *val, const struct kernel_param *kp)

static int get_memmap_mode(char *buffer, const struct kernel_param *kp)
{
- if (*((int *)kp->arg) == MEMMAP_ON_MEMORY_FORCE)
- return sprintf(buffer, "force\n");
- return param_get_bool(buffer, kp);
+ int mode = *((int *)kp->arg);
+
+ if (mode == MEMMAP_ON_MEMORY_FORCE)
+ return sprintf(buffer, "force\n");
+ return sprintf(buffer, "%c\n", mode ? 'Y' : 'N');
}

static const struct kernel_param_ops memmap_mode_ops = {
--
2.40.1



2024-01-10 16:16:06

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH] mm/memory_hotplug: fix memmap_on_memory sysfs value retrieval

On 10.01.24 15:01, Sumanth Korikkar wrote:
> The set_memmap_mode() function stores the kernel parameter memmap mode
> as an integer. However, the get_memmap_mode() function utilizes
> param_get_bool() to fetch the value as a boolean, leading to potential
> endianness issue. On Big-endian architectures, the memmap_on_memory is
> consistently displayed as 'N' regardless of its actual status.
>
> To address this endianness problem, the solution involves obtaining the
> mode as an integer. This adjustment ensures the proper display of the
> memmap_on_memory parameter, presenting it as one of the following
> options: Force, Y, or N.
>
> Fixes: 2d1f649c7c08 ("mm/memory_hotplug: support memmap_on_memory when memmap is not aligned to pageblocks")
> Suggested-by: Gerald Schaefer <[email protected]>
> Cc: <[email protected]> # v6.6+
> Signed-off-by: Sumanth Korikkar <[email protected]>
> ---
> mm/memory_hotplug.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b944e8bf1911..707027f69150 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -101,9 +101,11 @@ static int set_memmap_mode(const char *val, const struct kernel_param *kp)
>
> static int get_memmap_mode(char *buffer, const struct kernel_param *kp)
> {
> - if (*((int *)kp->arg) == MEMMAP_ON_MEMORY_FORCE)
> - return sprintf(buffer, "force\n");
> - return param_get_bool(buffer, kp);
> + int mode = *((int *)kp->arg);
> +
> + if (mode == MEMMAP_ON_MEMORY_FORCE)
> + return sprintf(buffer, "force\n");
> + return sprintf(buffer, "%c\n", mode ? 'Y' : 'N');
> }

I was wondering if we want to return "Y" instead of "force". But using
force seems more consistent.

Acked-by: David Hildenbrand <[email protected]>

--
Cheers,

David / dhildenb