Received: by 2002:a05:6a10:a0d1:0:0:0:0 with SMTP id j17csp308456pxa;
        Fri, 31 Jul 2020 12:34:11 -0700 (PDT)
X-Google-Smtp-Source: ABdhPJxTwBxhJZIqPXXGecrQqV5klL8mOJo9gC9VIQWEewWX3SWjeH0JRMNFpIv2Uf5SDpKfp9qu
X-Received: by 2002:a17:906:c04f:: with SMTP id bm15mr5396384ejb.207.1596224051349;
        Fri, 31 Jul 2020 12:34:11 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; t=1596224051; cv=none;
        d=google.com; s=arc-20160816;
        b=idGGOWw+AorzdyfG3Yc17U35S1b4I7vS3Y4MT4/+/rBFkNYuPkRHzMIrIKwXGUfke3
         D1CHATV19Q2LYX8IFO+cTd+2nY7dZGEn26DzYUvFJbR2c5YvpRb1HK8jJAGlPtvsOJCM
         CRkom4OW9zwYTYcbHkeo0mzjVASHuWpxkyrwgnHS84ZGQ1VOQGhC6HzvWphPbIHk3oF3
         hRmEBGwWRTb5J0ScfbhVd7Qlg0FKffdC95Zjjen6/dZaYQfDBNaL5Alg0FOLwMNhHONr
         HJwE67ecz5tTvkrVfq/1C8TX0BBwFu42boTzT6BGqHL6Bs+4+ztUrw4ZxCJdpYfdUhBM
         MDYg==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816;
        h=list-id:precedence:sender:dkim-signature:content-transfer-encoding
         :content-language:in-reply-to:mime-version:user-agent:date
         :message-id:from:references:cc:to:subject;
        bh=MF4Np6kSGoTk93XreGGnpf8Lflbk/aQSd9CdXlm3gxg=;
        b=GeD9MwbsyGfoxTNy7Pod4wsE9c3FbLNOvj/3mgYCiA2kX3QyK42/egwVN5GZlkBwgR
         LSz4GYlhzuaY+sDt3zLlcYuxfLaUxrjvmEjf+W8lwqoyK0m46QtKSujyPRwfrsDn5RUo
         ENBEeQm/MOcSMZoX8MtuW86D2iA8s4DsqGDPKEzginhpU36fT/ENZWKsxFZ4xZQ57A0n
         5KnTndFtjz0F+9F4NqrLtt8zA7/SCNJu1mu4Nq29Q2KuSWZa8U+kyk3AgemtWYh8Dqu5
         0KeunQWrGnqULPx71Qqok9ySgZ5J0Gp3jJte+cguY5M0f97VI0P3xcLJg0rS7xF4eX5l
         lHrA==
ARC-Authentication-Results: i=1; mx.google.com;
       dkim=pass header.i=@nvidia.com header.s=n1 header.b=Jrvy3o8X;
       spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18])
        by mx.google.com with ESMTP id o6si5559968edv.566.2020.07.31.12.33.46;
        Fri, 31 Jul 2020 12:34:11 -0700 (PDT)
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;
Authentication-Results: mx.google.com;
       dkim=pass header.i=@nvidia.com header.s=n1 header.b=Jrvy3o8X;
       spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
       dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1729209AbgGaTbZ (ORCPT <rfc822;vinadhy@gmail.com> + 99 others);
        Fri, 31 Jul 2020 15:31:25 -0400
Received: from hqnvemgate24.nvidia.com ([216.228.121.143]:5286 "EHLO
        hqnvemgate24.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726726AbgGaTbY (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 31 Jul 2020 15:31:24 -0400
Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate24.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)
        id <B5f24712e0000>; Fri, 31 Jul 2020 12:29:50 -0700
Received: from hqmail.nvidia.com ([172.20.161.6])
  by hqpgpgate101.nvidia.com (PGP Universal service);
  Fri, 31 Jul 2020 12:31:24 -0700
X-PGP-Universal: processed;
        by hqpgpgate101.nvidia.com on Fri, 31 Jul 2020 12:31:24 -0700
Received: from rcampbell-dev.nvidia.com (172.20.13.39) by HQMAIL107.nvidia.com
 (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 31 Jul
 2020 19:31:23 +0000
Subject: Re: [PATCH v4 6/6] mm/migrate: remove range invalidation in
 migrate_vma_pages()
To:     Jason Gunthorpe <jgg@ziepe.ca>
CC:     <linux-rdma@vger.kernel.org>, <linux-mm@kvack.org>,
        <nouveau@lists.freedesktop.org>, <kvm-ppc@vger.kernel.org>,
        <linux-kselftest@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        "Jerome Glisse" <jglisse@redhat.com>,
        John Hubbard <jhubbard@nvidia.com>,
        "Christoph Hellwig" <hch@lst.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        Shuah Khan <shuah@kernel.org>, Ben Skeggs <bskeggs@redhat.com>,
        Bharata B Rao <bharata@linux.ibm.com>
References: <20200723223004.9586-1-rcampbell@nvidia.com>
 <20200723223004.9586-7-rcampbell@nvidia.com>
 <20200728191940.GB159104@nvidia.com>
 <7f947311-0034-9148-1dca-fb9b9a10abc4@nvidia.com>
 <20200731191543.GJ24045@ziepe.ca>
X-Nvconfidentiality: public
From:   Ralph Campbell <rcampbell@nvidia.com>
Message-ID: <b8c35d45-91d0-1c99-390a-049867af17fb@nvidia.com>
Date:   Fri, 31 Jul 2020 12:31:23 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.2
MIME-Version: 1.0
In-Reply-To: <20200731191543.GJ24045@ziepe.ca>
X-Originating-IP: [172.20.13.39]
X-ClientProxiedBy: HQMAIL107.nvidia.com (172.20.187.13) To
 HQMAIL107.nvidia.com (172.20.187.13)
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
        t=1596223790; bh=MF4Np6kSGoTk93XreGGnpf8Lflbk/aQSd9CdXlm3gxg=;
        h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From:
         Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:
         X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language:
         Content-Transfer-Encoding;
        b=Jrvy3o8XJ2rY3h6bMQqWanvXTXrCvZz2LWHdo1X7SNs+nmekjNPqnyIhs7UtxJkuW
         FMnHvtBQj8zr9jesv11SxggwiWhDu6842G9JagwXSjMizmbF/C+x2Ml8s6IOQs08nq
         2n8TQS51oUHkTQcN2FCe7qo70Q8VKNVlgQ4qLtBkSQ9+ib1yW6Nc70SFp/jQcRjyxh
         WxtFmZ16kXYh+Hq3mhLNHf/Y0PI4F+dahDYQaKaTitPTDBPbVjT0OVveLaQMuLEI/Z
         pIPJrtAiTRbfiY7Pb5dn/ns2bCBOZa3fWf9TQyI6WuLHb8Jv/Fs6eww3WhAufsO/x5
         njIu1bRlWI0iw==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/31/20 12:15 PM, Jason Gunthorpe wrote:
> On Tue, Jul 28, 2020 at 03:04:07PM -0700, Ralph Campbell wrote:
>>
>> On 7/28/20 12:19 PM, Jason Gunthorpe wrote:
>>> On Thu, Jul 23, 2020 at 03:30:04PM -0700, Ralph Campbell wrote:
>>>> When migrating the special zero page, migrate_vma_pages() calls
>>>> mmu_notifier_invalidate_range_start() before replacing the zero page
>>>> PFN in the CPU page tables. This is unnecessary since the range was
>>>> invalidated in migrate_vma_setup() and the page table entry is checked
>>>> to be sure it hasn't changed between migrate_vma_setup() and
>>>> migrate_vma_pages(). Therefore, remove the redundant invalidation.
>>>
>>> I don't follow this logic, the purpose of the invalidation is also to
>>> clear out anything that may be mirroring this VA, and "the page hasn't
>>> changed" doesn't seem to rule out that case?
>>>
>>> I'm also not sure I follow where the zero page came from?
>>
>> The zero page comes from an anonymous private VMA that is read-only
>> and the user level CPU process tries to read the page data (or any
>> other read page fault).
>>
>>> Jason
>>>
>>
>> The overall migration process is:
>>
>> mmap_read_lock()
>>
>> migrate_vma_setup()
>>        // invalidates range, locks/isolates pages, puts migration entry in page table
>>
>> <driver allocates destination pages and copies source to dest>
>>
>> migrate_vma_pages()
>>        // moves source struct page info to destination struct page info.
>>        // clears migration flag for pages that can't be migrated.
>>
>> <driver updates device page tables for pages still migrating, rollback pages not migrating>
>>
>> migrate_vma_finalize()
>>        // replaces migration page table entry with destination page PFN.
>>
>> mmap_read_unlock()
>>
>> Since the address range is invalidated in the migrate_vma_setup() stage,
>> and the page is isolated from the LRU cache, locked, unmapped, and the page table
>> holds a migration entry (so the page can't be faulted and the CPU page table set
>> valid again), and there are no extra page references (pins), the page
>> "should not be modified".
> 
> That is the physical page though, it doesn't prove nobody else is
> reading the PTE.
>   
>> For pte_none()/is_zero_pfn() entries, migrate_vma_setup() leaves the
>> pte_none()/is_zero_pfn() entry in place but does still call
>> mmu_notifier_invalidate_range_start() for the whole range being migrated.
> 
> Ok..
> 
>> In the migrate_vma_pages() step, the pte page table is locked and the
>> pte entry checked to be sure it is still pte_none/is_zero_pfn(). If not,
>> the new page isn't inserted. If it is still none/zero, the new device private
>> struct page is inserted into the page table, replacing the pte_none()/is_zero_pfn()
>> page table entry. The secondary MMUs were already invalidated in the migrate_vma_setup()
>> step and a pte_none() or zero page can't be modified so the only invalidation needed
>> is the CPU TLB(s) for clearing the special zero page PTE entry.
> 
> No, the secondary MMU was invalidated but the invalidation start/end
> range was exited. That means a secondary MMU is immeidately able to
> reload the zero page into its MMU cache.
> 
> When this code replaces the PTE that has a zero page it also has to
> invalidate again so that secondary MMU's are guaranteed to pick up the
> new PTE value.
> 
> So, I still don't understand how this is safe?
> 
> Jason

Oops, you are right of course. I was only thinking of the device doing the migration
and forgetting about a second device faulting on the same page.
You can drop patch from the series.