Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200915145040.GA2949@xz-x1> <20200915160553.GJ1221970@ziepe.ca>
 <20200915182933.GM1221970@ziepe.ca> <20200915191346.GD2949@xz-x1>
 <20200915193838.GN1221970@ziepe.ca> <20200915213330.GE2949@xz-x1>
 <20200915232238.GO1221970@ziepe.ca> <e6c352f8-7ee9-0702-10a4-122d2c4422fc@nvidia.com>
 <20200916174804.GC8409@ziepe.ca> <20200916184619.GB40154@xz-x1> <20200917112538.GD8409@ziepe.ca>
In-Reply-To: <20200917112538.GD8409@ziepe.ca>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 17 Sep 2020 11:11:06 -0700
Message-ID: <CAHk-=wjtfjB3TqTFRzVmOrB9Mii6Yzc-=wKq0fu4ruDE6AsJgg@mail.gmail.com>
Subject: Re: [PATCH 1/4] mm: Trial do_wp_page() simplification
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     Peter Xu <peterx@redhat.com>, 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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> I botched the last version of the patch, here is something a bit
> better.

So I'd like to understand why this problem happens.

Myt argument to Hugh a few weeks ago was that page pinning should take
care of all this:

 (a) if the pinner is going to change the page, it will have to get
the pin with FOLL_WRITE in addition to FOLL_PIN

 (b) that means it will do the COW and mark the page writable and dirty

 (c) the whole _point_ of the FOLL_PIN is that subsequent operations
shouldn't make it non-writable any more (ie it can't be unmapped, and
we should synchronize on fork etc)

So I get the strong feeling that this whole approach to "fix" COW is
really papering over the real problem.

To me, the whole point of pinning is to avoid issues like this. If it
didn't fix this issue, then why did we go to all the effort of
treating pinned pages differently?

Your patch may avoid the problem, but I think it implies things are
horribly horribly broken in pinning land.

I also note that the _only_ user of page_maybe_dma_pinned() right now
is a debug check in gup_benchmark. I think your patch may _work_, and
I think that thanks to page_maybe_dma_pinned() it might even avoid the
horrible case, but I feel we should just truly fix pinning to be
meaningful instead.

IOW, in addition to keeping the page mapped, it should keep the page writable.

Why isn't it?

                     Linus