On 01.08.23 19:04, Peter Xu wrote:
On Tue, Aug 01, 2023 at 06:15:48PM +0200, David Hildenbrand wrote:
On 01.08.23 17:48, Peter Xu wrote:
On Tue, Aug 01, 2023 at 02:48:37PM +0200, David Hildenbrand wrote:
@@ -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;
Since at it, do we want to not set it for FOLL_REMOTE, which still sounds like a good thing to have?
I thought about that, but decided against making that patch here more complicated to eventually rip it again all out in #4.
I thought that was the whole point of having patch 4 separate, because we should assume patch 4 may not exist in (at least) some trees, so I ignored patch 4 when commenting here, and we should not assume it's destined to be removed here.
For me, the goal of this patch is to bring it *as close as possible* to the previous state as before, so we can backport it to stable without too many surprises (effectively, only a handful of FOLL_FORCE/ptrace user will get a different behavior).
I could add a separate patch that does the FOLL_REMOTE thing, but then, maybe we should do that if patch #4 runs into real trouble :)
But no strong opinion if this is what everybody wants in this patch.
I fully agree that FOLL_REMOTE does not make too much sense, but let's rather keep it simple for this patch.
It's fine - I suppose this patch fixes whatever we're aware of that's broken with FOLL_NUMA's removal, so it can even be anything on top when needed. I assume I'm happy to ack this with/without that change, then:
Acked-by: Peter Xu peterx@redhat.com
Thanks!
PS: I still hope that the other oneliner can be squashed here directly; it literally changes exact the same line above so reading this patch alone can be affected. You said there you didn't want the commit message to be too long here, but this is definitely not long at all! I bet you have similar understanding to me on defining "long commit message". :) I'd never worry that. Your call.
No strong opinion, it just felt cleaner to not start adding what I have in that separate patch commit message in here.