Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <75e721cb-18c2-4717-bdbe-af346b1deafa@arm.com>
Date:   Thu, 16 Nov 2023 11:13:14 +0000
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 01/14] mm: Batch-copy PTE ranges during fork()
Content-Language: en-GB
To:     David Hildenbrand <david@redhat.com>,
        kernel test robot <lkp@intel.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Marc Zyngier <maz@kernel.org>,
        Oliver Upton <oliver.upton@linux.dev>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Matthew Wilcox <willy@infradead.org>,
        Yu Zhao <yuzhao@google.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        John Hubbard <jhubbard@nvidia.com>, Zi Yan <ziy@nvidia.com>
Cc:     oe-kbuild-all@lists.linux.dev,
        Linux Memory Management List <linux-mm@kvack.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20231115163018.1303287-2-ryan.roberts@arm.com>
 <202311160516.kHhfmjvl-lkp@intel.com>
 <e6060ff2-ced0-42fd-b92a-c2e710c4e15a@arm.com>
 <4e8d329c-eda6-4ff8-bb56-8924bb4583b2@redhat.com>
 <a397d45b-6369-4a45-915e-cbebecd4556b@arm.com>
 <930b5f6a-27d9-43da-bf9f-1478c8de1af8@redhat.com>
From:   Ryan Roberts <ryan.roberts@arm.com>
In-Reply-To: <930b5f6a-27d9-43da-bf9f-1478c8de1af8@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 16/11/2023 11:01, David Hildenbrand wrote:
> On 16.11.23 11:36, Ryan Roberts wrote:
>> On 16/11/2023 10:12, David Hildenbrand wrote:
>>> On 16.11.23 11:07, Ryan Roberts wrote:
>>>> Hi All,
>>>>
>>>> Hoping for some guidance below!
>>>>
>>>>
>>>> On 15/11/2023 21:26, kernel test robot wrote:
>>>>> Hi Ryan,
>>>>>
>>>>> kernel test robot noticed the following build errors:
>>>>>
>>>>> [auto build test ERROR on akpm-mm/mm-everything]
>>>>> [also build test ERROR on linus/master v6.7-rc1 next-20231115]
>>>>> [cannot apply to arm64/for-next/core efi/next]
>>>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>>>> And when submitting patch, we suggest to use '--base' as documented in
>>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>>
>>>>> url:
>>>>> https://github.com/intel-lab-lkp/linux/commits/Ryan-Roberts/mm-Batch-copy-PTE-ranges-during-fork/20231116-010123
>>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git
>>>>> mm-everything
>>>>> patch link:
>>>>> https://lore.kernel.org/r/20231115163018.1303287-2-ryan.roberts%40arm.com
>>>>> patch subject: [PATCH v2 01/14] mm: Batch-copy PTE ranges during fork()
>>>>> config: arm-randconfig-002-20231116
>>>>> (https://download.01.org/0day-ci/archive/20231116/202311160516.kHhfmjvl-lkp@intel.com/config)
>>>>> compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
>>>>> reproduce (this is a W=1 build):
>>>>> (https://download.01.org/0day-ci/archive/20231116/202311160516.kHhfmjvl-lkp@intel.com/reproduce)
>>>>>
>>>>> If you fix the issue in a separate patch/commit (i.e. not just a new
>>>>> version of
>>>>> the same patch/commit), kindly add following tags
>>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>>> | Closes:
>>>>> https://lore.kernel.org/oe-kbuild-all/202311160516.kHhfmjvl-lkp@intel.com/
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>      mm/memory.c: In function 'folio_nr_pages_cont_mapped':
>>>>>>> mm/memory.c:969:16: error: implicit declaration of function 'pte_pgprot';
>>>>>>> did you mean 'ptep_get'? [-Werror=implicit-function-declaration]
>>>>>        969 |         prot = pte_pgprot(pte_mkold(pte_mkclean(ptent)));
>>>>>            |                ^~~~~~~~~~
>>>>>            |                ptep_get
>>>>>      cc1: some warnings being treated as errors
>>>>
>>>> It turns out that pte_pgprot() is not universal; its only implemented by
>>>> architectures that select CONFIG_HAVE_IOREMAP_PROT (currently arc, arm64,
>>>> loongarch, mips, powerpc, s390, sh, x86).
>>>>
>>>> I'm using it in core-mm to help calculate the number of "contiguously mapped"
>>>> pages within a folio (note that's not the same as arm64's notion of
>>>> contpte-mapped. I just want to know that there are N physically contiguous
>>>> pages
>>>> mapped virtually contiguously with the same permissions). And I'm using
>>>> pte_pgprot() to extract the permissions for each pte to compare. It's important
>>>> that we compare the permissions because just because the pages belongs to the
>>>> same folio doesn't imply they are mapped with the same permissions; think
>>>> mprotect()ing a sub-range.
>>>>
>>>> I don't have a great idea for how to fix this - does anyone have any thoughts?
>>>
>>> KIS :) fork() operates on individual VMAs if I am not daydreaming.
>>>
>>> Just check for the obvious pte_write()/dirty/ and you'll be fine.
>>
>> Yes, that seems much simpler! I think we might have to be careful about the uffd
>> wp bit too? I think that's it - are there any other exotic bits that might need
>> to be considered?
> 
> Good question. Mimicing what the current code already does should be sufficient.
> uffd-wp should have the PTE R/O. You can set the contpte bit independent of any
> SW bit (uffd-wp, softdirty, ...) I guess, no need to worry about that.
> 

OK thanks. I'll rework for this approach in v3.