Re: [PATCH v2 01/14] mm: Batch-copy PTE ranges during fork()

[David Hildenbrand]

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@xxxxxxxxx/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@xxxxxxxxx/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@xxxxxxxxx>
> > > > | Closes:
> > > > https://lore.kernel.org/oe-kbuild-all/202311160516.kHhfmjvl-lkp@xxxxxxxxx/
> > > >
> > > > 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.

--
Cheers,

David / dhildenb