Subject: Re: [PATCH] arm64: Change elfcore for_each_mte_vma() to use VMA iterator

From: Catalin Marinas <[email-masked]>
Date: Tue, 22 Feb 2022 17:26:35 +0000

On Tue, Feb 22, 2022 at 04:20:16PM +0000, Will Deacon wrote:
> On Tue, Feb 22, 2022 at 02:26:03PM +0000, Liam Howlett wrote:
> > * Catalin Marinas <[email-masked]> [220221 13:07]:
> > > On Fri, Feb 18, 2022 at 02:37:04AM +0000, Liam Howlett wrote:
> > > > diff --git a/arch/arm64/kernel/elfcore.c b/arch/arm64/kernel/elfcore.c
> > > > index 3455ee4acc04..930a0bc4cac4 100644
> > > > --- a/arch/arm64/kernel/elfcore.c
> > > > +++ b/arch/arm64/kernel/elfcore.c
> > > > @@ -8,9 +8,9 @@
> > > >  #include <asm/cpufeature.h>
> > > >  #include <asm/mte.h>
> > > >  
> > > > -#define for_each_mte_vma(tsk, vma)					\
> > > > +#define for_each_mte_vma(vmi, vma)					\
> > > >  	if (system_supports_mte())					\
> > > > -		for (vma = tsk->mm->mmap; vma; vma = vma->vm_next)	\
> > > > +		for_each_vma(vmi, vma)					\
> > > >  			if (vma->vm_flags & VM_MTE)
> > > >  
> > > >  static unsigned long mte_vma_tag_dump_size(struct vm_area_struct *vma)
> > > > @@ -65,8 +65,9 @@ Elf_Half elf_core_extra_phdrs(void)
> > > >  {
> > > >  	struct vm_area_struct *vma;
> > > >  	int vma_count = 0;
> > > > +	VMA_ITERATOR(vmi, current->mm, 0);
> > > >  
> > > > -	for_each_mte_vma(current, vma)
> > > > +	for_each_mte_vma(vmi, vma)
> > > >  		vma_count++;
> > > 
> > > I'm fine with the patch but it can't be applied to arm64 for-next/mte
> > > branch as it won't build and the maple tree doesn't have the MTE
> > > patches. Do you have a stable branch with the for_each_vma() iterator?
> > 
> > The vma iterator uses the maple tree, so this patch would resolve the
> > conflict but both branches are needed.
> 
> I'm not really sure what to do here, then. I think the conflict is nasty
> enough that we should resolve it before the trees reach Linus, but there
> doesn't seem to be a way forward other than one of us merging the other
> branch. I'd like to avoid having MTE coredump support depend on the maple
> tree work.
> 
> Is there some way you could provide a branch which implements
> for_each_vma() using the old vma list, and then the maple tree series
> could switch that over to the maple tree without breaking things?

Without a branch, we could apply something like below on top of Liam's
patch and revert it once the maple tree is upstream:

diff --git a/arch/arm64/kernel/elfcore.c b/arch/arm64/kernel/elfcore.c
index 930a0bc4cac4..400ec7a902df 100644
--- a/arch/arm64/kernel/elfcore.c
+++ b/arch/arm64/kernel/elfcore.c
@@ -8,6 +8,13 @@
 #include <asm/cpufeature.h>
 #include <asm/mte.h>

+#ifndef VMA_ITERATOR
+#define VMA_ITERATOR(name, mm, addr) \
+	struct mm_struct *name = mm
+#define for_each_vma(vmi, vma) \
+	for (vma = vmi->mmap; vma; vma = vma->vm_next)
+#endif
+
 #define for_each_mte_vma(vmi, vma)					\
 	if (system_supports_mte())					\
 		for_each_vma(vmi, vma)					\

-- 
Catalin