On Fri, Jun 19, 2020 at 5:59 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> Commit e900a918b098 ("mm: shuffle initial free memory to improve
> memory-side-cache utilization") promised "autodetection of a
> memory-side-cache (to be added in a follow-on patch)" over a year ago.
>
> The original series included patches [1], however, they were dropped
> during review [2] to be followed-up later.
>
> Due to lack of platforms that publish an HMAT, autodetection is currently
> not implemented. However, manual activation is actively used [3]. Let's
> simplify for now and re-add when really (ever?) needed.
>
> [1] https://lkml.kernel.org/r/154510700291.1941238.817190985966612531.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> [2] https://lkml.kernel.org/r/154690326478.676627.103843791978176914.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> [3] https://lkml.kernel.org/r/CAPcyv4irwGUU2x+c6b4L=KbB1dnasNKaaZd6oSpYjL9kfsnROQ@xxxxxxxxxxxxxx
>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Minchan Kim <minchan@xxxxxxxxxx>
> Cc: Huang Ying <ying.huang@xxxxxxxxx>
> Cc: Wei Yang <richard.weiyang@xxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> mm/shuffle.c | 28 ++--------------------------
> mm/shuffle.h | 17 -----------------
> 2 files changed, 2 insertions(+), 43 deletions(-)
>
> diff --git a/mm/shuffle.c b/mm/shuffle.c
> index dd13ab851b3ee..9b5cd4b004b0f 100644
> --- a/mm/shuffle.c
> +++ b/mm/shuffle.c
> @@ -10,33 +10,11 @@
> #include "shuffle.h"
>
> DEFINE_STATIC_KEY_FALSE(page_alloc_shuffle_key);
> -static unsigned long shuffle_state __ro_after_init;
> -
> -/*
> - * Depending on the architecture, module parameter parsing may run
> - * before, or after the cache detection. SHUFFLE_FORCE_DISABLE prevents,
> - * or reverts the enabling of the shuffle implementation. SHUFFLE_ENABLE
> - * attempts to turn on the implementation, but aborts if it finds
> - * SHUFFLE_FORCE_DISABLE already set.
> - */
> -__meminit void page_alloc_shuffle(enum mm_shuffle_ctl ctl)
> -{
> - if (ctl == SHUFFLE_FORCE_DISABLE)
> - set_bit(SHUFFLE_FORCE_DISABLE, &shuffle_state);
> -
> - if (test_bit(SHUFFLE_FORCE_DISABLE, &shuffle_state)) {
> - if (test_and_clear_bit(SHUFFLE_ENABLE, &shuffle_state))
> - static_branch_disable(&page_alloc_shuffle_key);
> - } else if (ctl == SHUFFLE_ENABLE
> - && !test_and_set_bit(SHUFFLE_ENABLE, &shuffle_state))
> - static_branch_enable(&page_alloc_shuffle_key);
> -}
>
> static bool shuffle_param;
> static int shuffle_show(char *buffer, const struct kernel_param *kp)
> {
> - return sprintf(buffer, "%c\n", test_bit(SHUFFLE_ENABLE, &shuffle_state)
> - ? 'Y' : 'N');
> + return sprintf(buffer, "%c\n", shuffle_param ? 'Y' : 'N');
> }
>
> static __meminit int shuffle_store(const char *val,
> @@ -47,9 +25,7 @@ static __meminit int shuffle_store(const char *val,
> if (rc < 0)
> return rc;
> if (shuffle_param)
> - page_alloc_shuffle(SHUFFLE_ENABLE);
> - else
> - page_alloc_shuffle(SHUFFLE_FORCE_DISABLE);
> + static_branch_enable(&page_alloc_shuffle_key);
> return 0;
> }
Let's do proper input validation here and require 1 / 'true' to enable
shuffling and not also allow 0 to be an 'enable' value.
Other than that looks like the right move to me until end users or
distros start asking for the kernel to do this by default, I'm not
aware of any of those requests to date. People seem fine to set the
boot option.
After the above fixups you can add:
Acked-by: Dan Williams <dan.j.williams@xxxxxxxxx>
