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

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Peter Xu <peterx@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
	John Hubbard <jhubbard@nvidia.com>,
	 Leon Romanovsky <leonro@nvidia.com>,
	Linux-MM <linux-mm@kvack.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"Maya B . Gokhale" <gokhale2@llnl.gov>,
	 Yang Shi <yang.shi@linux.alibaba.com>,
	Marty Mcfadden <mcfadden8@llnl.gov>,
	 Kirill Shutemov <kirill@shutemov.name>,
	Oleg Nesterov <oleg@redhat.com>, Jann Horn <jannh@google.com>,
	 Jan Kara <jack@suse.cz>, Kirill Tkhai <ktkhai@virtuozzo.com>,
	 Andrea Arcangeli <aarcange@redhat.com>,
	Christoph Hellwig <hch@lst.de>,
	 Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/4] mm: Trial do_wp_page() simplification
Date: Thu, 17 Sep 2020 12:42:11 -0700	[thread overview]
Message-ID: <CAHk-=wgw3GNyF_6euymOFxM62Y3B=C=f2iUJn8Py-u5YELJ5JA@mail.gmail.com> (raw)
In-Reply-To: <20200917190332.GB133226@xz-x1>

On Thu, Sep 17, 2020 at 12:03 PM Peter Xu <peterx@redhat.com> wrote:
>
> The fork() should be slightly slower though, since we'll need to copy the data
> for all the DMA buffers for each of the child processes, even if we should be
> pretty sure those processes won't use these pages at all.

I think that as long as we only trigger for pinned pages then that's
fine - and in fact I think we might want to add a WARN_ON_ONCE() or
something to it if we are sure enough about that page pinning.

Because the whole "do page pinning without MADV_DONTFORK and then fork
the area" is I feel a very very invalid load. It sure as hell isn't
something we should care about performance for, and in fact it is
something we should very well warn for exactly to let people know
"this process is doing bad things".

My main worry is that page_maybe_dma_pinned() not being exact. I'm not
entirely convinced whether it really is extremely rare or not.

I could well imagine some very fork-happy load having very elevated
page counts (exactly *because* it is fork-happy), and then the
performance thing might in fact be an issue.

That said, you really have to be *very* fork-happy for this to
trigger, with GUP_PIN_COUNTING_BIAS being 1024. Those kinds of
fork-happy loads very much used to exist, but I think anybody who
cares about performance will have long long since switched to
threading, not forking.

Do people fork a thousand children? Sure. Is it something we need to
worry about a lot? I don't know. I'm thinking ot the android zygote
disaster..

Is there possibly somethign else we can filter on than just
GUP_PIN_COUNTING_BIAS? Because it could be as simple as just marking
the vma itself and saying "this vma has had a page pinning event done
on it".

Because if we only start copying the page *iff* the vma is marked by
that "this vma had page pinning" _and_ the page count is bigger than
GUP_PIN_COUNTING_BIAS, than I think we can rest pretty easily knowing
that we aren't going to hit some regular old-fashioned UNIX server
cases with a lot of forks..

                       Linus