[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zx_JeZcugENuStCd@kernel.org>
Date: Mon, 28 Oct 2024 19:27:21 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Gregory Price <gourry@...rry.net>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, linux-mm@...ck.org, linux-cxl@...ck.org,
Jonathan.Cameron@...wei.com, dan.j.williams@...el.com,
rrichter@....com, Terry.Bowman@....com, dave.jiang@...el.com,
ira.weiny@...el.com, alison.schofield@...el.com,
dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, hpa@...or.com,
rafael@...nel.org, lenb@...nel.org, david@...hat.com,
osalvador@...e.de, gregkh@...uxfoundation.org,
akpm@...ux-foundation.org
Subject: Re: [PATCH v3 1/3] memory: implement
memory_block_advise/probe_max_size
Hi,
On Tue, Oct 22, 2024 at 05:34:48PM -0400, Gregory Price wrote:
> Hotplug memory sources may have opinions on what the memblock size
> should be - usually for alignment purposes. For example, CXL memory
> extents can be 256MB with a matching alignment. If this size/alignment
> is smaller than the block size, it can result in stranded capacity.
>
> Implement memory_block_advise_max_size for use prior to allocator init,
> for software to advise the system on the max block size.
>
> Implement memory_block_probe_max_size for use by arch init code to
> calculate the best block size. Use of advice is architecture defined.
>
> The probe value can never change after first probe. Calls to advise
> after probe will return -EBUSY to aid debugging.
>
> On systems without hotplug, always return -ENODEV and 0 respectively.
>
> Suggested-by: Ira Weiny <ira.weiny@...el.com>
> Signed-off-by: Gregory Price <gourry@...rry.net>
> ---
> drivers/base/memory.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/memory.h | 10 ++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 67858eeb92ed..a0fd9e1451aa 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -110,6 +110,49 @@ static void memory_block_release(struct device *dev)
> kfree(mem);
> }
>
> +/*
If the intention for this to be a kernel-doc comment it should start with
/**, should be placed right before the documented function and start with a
brief function description
> + * @size: suggestion for maximum block size. must be aligned on power of 2.
> + *
> + * Early boot software (pre-allocator init) may advise archs on the max block
> + * size.This value can only decrease after initialization, as the intent is to
> + * identify the largest supported alignment for all sources.
> + *
> + * Use of this value is arch dependent, as is min/max block size.
> + *
> + * Returns: 0 on success
> + * -EINVAL if size is 0 or not pow2 aligned
> + * -EBUSY if value has already been probed
> + */
> +static size_t memory_block_advised_sz;
> +static bool memory_block_size_probed;
> +int memory_block_advise_max_size(size_t bz)
size seems to me a better parameter name :)
> +{
> + if (!bz || (bz & (bz - 1)) != 0)
we have is_power_of_2()
> + return -EINVAL;
> +
> + if (memory_block_size_probed)
> + return -EBUSY;
> +
> + if (memory_block_advised_sz)
> + memory_block_advised_sz = min(bz, memory_block_advised_sz);
> + else
> + memory_block_advised_sz = bz;
> +
> + return 0;
> +}
> +
> +/*
should it be kernel-doc?
> + * memory_block_probe_max_size provides a suggested maximum memory block
> + * size value. After the first call, the value can never change.
> + *
> + * Returns: advised size in bytes, or 0 if never set.
> + */
> +size_t memory_block_probe_max_size(void)
> +{
> + memory_block_size_probed = true;
> + return memory_block_advised_sz;
> +}
> +
> unsigned long __weak memory_block_size_bytes(void)
> {
> return MIN_MEMORY_BLOCK_SIZE;
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index c0afee5d126e..47c00d6e1165 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -149,6 +149,14 @@ static inline int hotplug_memory_notifier(notifier_fn_t fn, int pri)
> {
> return 0;
> }
> +static inline int memory_block_advise_max_size(size_t size)
> +{
> + return -ENODEV;
> +}
> +static inline size_t memory_block_probe_max_size(void)
> +{
> + return 0;
> +}
> #else /* CONFIG_MEMORY_HOTPLUG */
> extern int register_memory_notifier(struct notifier_block *nb);
> extern void unregister_memory_notifier(struct notifier_block *nb);
> @@ -181,6 +189,8 @@ int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t func,
> void memory_block_add_nid(struct memory_block *mem, int nid,
> enum meminit_context context);
> #endif /* CONFIG_NUMA */
> +int memory_block_advise_max_size(size_t size);
> +size_t memory_block_probe_max_size(void);
> #endif /* CONFIG_MEMORY_HOTPLUG */
>
> /*
> --
> 2.43.0
>
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists