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

Linus Torvalds
2020-09-15T00:19:29Z

On Mon, Sep 14, 2020 at 4:28 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> Hmm. If symptoms stop with this patch should we investigate
> MADV_DONTFORK?

I took a quick look at it, and it all looks trivially correct.

All MADV_DONTFORK does is to set the VM_DONTCOPY flag in the vma.

And dup_mmap() in kernel/fork.c is very trivial, and does

if (mpnt->vm_flags & VM_DONTCOPY) {
vm_stat_account(mm, mpnt->vm_flags, -vma_pages(mpnt));
continue;
}

for a vma that has that VM_DONTCOPY flag.

So I don't think it's MADV_DONTFORK, and in fact if that _had_ been
broken, then the old "look at page_mapcount()" would have shown the
problem too, since by definition a fork() would have increased that
count.

That said, the thing Hugh worried about was random other VM-internal
reasons why the page flags end up being elevated, that aren't due to
these things. And he's right. The new aggressive COW by that
do_wp_page() simplification will basically COW for any random thing.

My argument to Hugh was that if the page has become private to a
single mapping, even if it has its count elevated it should all simply
be writable, ie it shouldn't have gotten the paeg fault that causes
the COW in the first place. IOW, my thinking was that any proper page
pinning will also have to make sure that the page is already writable
and dirty, so no amount of other page counts should even matter.

But that argument may be entirely bogus, because I didn't think of
case "insert random case here".

My original force-COW patch for GUP avoided this issue, exactly
because it basically said that a GUP is a write - so it didn't care
about whatever state the page had, it *forced* the page to be mapped
dirty and writable in the target.

But part of the argument for the do_wp_page() simplification thing was
that it allowed us to remove my force-COW thing.

Is there perhaps some easy test-case that shows this that doesn't
require any actual rdma hardware?

Linus