Re: [PATCH 1/4] mm: Trial do_wp_page() simplification

From: Linus Torvalds
Date: Fri Sep 18 2020 - 17:07:26 EST


On Fri, Sep 18, 2020 at 1:41 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> What would be the result if we simply use GFP_ATOMIC? Would there be too many
> pages to allocate in bulk for ATOMIC?

It's very easy to run out of memory with GFP_ATOMIC, and also cause
various nasty issues with networking (ie when you've depleted atomic
memory, networking starts losing packets etc).

So yeah, this code should not use GFP_ATOMIC, I think it just needs to
drop and re-take the paeg table lock.

Which is easy enough to do: returning a zero 'entry.val' already does
that for other reasons, there's nothing magical about holding the lock
here, there's no larger page table lock requirement.

The main annoyance is that I think it means that copy_pte_range()
would also have to have a special "preallocation page" thing for this
case, so that it can drop the lock, do the allocation, and then take
the lock again and return 0 (to repeat - now with the preallocation
filled).

Honestly, if we had a completely *reliable* sign of "this page is
pinned", then I think the much nicer option would be to just say
"pinned pages will not be copied at all". Kind of an implicit
VM_DONTCOPY.

(Or we'd do the reverse, and say "pinned pages stay pinned even in the child").

But that's not an option when the pinning test is a "maybe". Oh well.

Linus