Re: [PATCH v2 1/8] mm/gup: reintroduce FOLL_NUMA as FOLL_HONOR_NUMA_FAULT

From: David Hildenbrand
Date: Wed Aug 02 2023 - 11:13:26 EST


Reported-by: liubo <liubo254@xxxxxxxxxx>
Closes: https://lore.kernel.org/r/20230726073409.631838-1-liubo254@xxxxxxxxxx
Reported-by: Peter Xu <peterx@xxxxxxxxxx>
Closes: https://lore.kernel.org/all/ZMKJjDaqZ7FW0jfe@x1n/
Fixes: 474098edac26 ("mm/gup: replace FOLL_NUMA by gup_can_follow_protnone()")
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>

I agree that FOLL_REMOTE probably needs separate treatment but also agree
that it's outside the context of this patch, particularly as a -stable
candidate so

Acked-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>

I've a minor nit below that would be nice to get fixed up, but not
mandatory.

Thanks Mel for taking a look, so I don't mess up once more :)


---
include/linux/mm.h | 21 +++++++++++++++------
include/linux/mm_types.h | 9 +++++++++
mm/gup.c | 29 +++++++++++++++++++++++------
mm/huge_memory.c | 2 +-
4 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 2493ffa10f4b..f463d3004ddc 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2240,6 +2244,12 @@ static bool is_valid_gup_args(struct page **pages, int *locked,
gup_flags |= FOLL_UNLOCKABLE;
}
+ /*
+ * For now, always trigger NUMA hinting faults. Some GUP users like
+ * KVM really require it to benefit from autonuma.
+ */
+ gup_flags |= FOLL_HONOR_NUMA_FAULT;
+
/* FOLL_GET and FOLL_PIN are mutually exclusive. */
if (WARN_ON_ONCE((gup_flags & (FOLL_PIN | FOLL_GET)) ==
(FOLL_PIN | FOLL_GET)))

Expand on *why* KVM requires it even though I suspect this changes later
in the series. Maybe "Some GUP users like KVM require the hint to be as
the calling context of GUP is functionally similar to a memory reference
from task context"?

It's raised later in this series but it doesn't hurt to discuss it here in a bit more detail.

Sounds good to me.


Also minor nit -- s/autonuma/NUMA Balancing/ or numab. autonuma refers to
a specific implementation of automatic balancing that operated similar to
khugepaged but not merged. If you grep for it, you'll find that autonuma
is only referenced in powerpc-specific code. It's not important these
days but very early on, it was very confusing if AutoNUMA was mentioned
when NUMAB was intended.

Ah, yes, thanks. That's the one of the only place where that terminology accidentally slipped in.

I'll wait for more feedback and resend!

--
Cheers,

David / dhildenb