Subject: Re: linux-next: build failure after merge of the akpm tree

From: Stephen Rothwell <[email-masked]>
Date: Fri, 21 Feb 2020 10:18:45 +1100

Hi all,

On Sun, 16 Feb 2020 22:45:35 -0800 Arjun Roy <[email-masked]> wrote:
>
> On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <[email-masked]> wrote:
> >
> > On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <[email-masked]> wrote:  
> > >
> > > After merging the akpm tree, today's linux-next build (sparc64 defconfig)
> > > failed like this:
> > >
> > > mm/memory.c: In function 'insert_pages':
> > > mm/memory.c:1523:56: error: macro "pte_index" requires 2 arguments, but only 1 given
> > >    remaining_pages_total, PTRS_PER_PTE - pte_index(addr));
> > >                                                         ^
> > >
> > > Caused by commit
> > >
> > >   366142f0b000 ("mm/memory.c: add vm_insert_pages()")
> > >
> > > This is the first use of pte_index() outside arch specific code and the
> > > sparc64 version of pte_index() nas an extra argument.
> >
> > Looks like this happens for sparc, and also metag. Other platforms
> > just take the addr parameter based on a quick search.
> 
> And actually I guess there's no metag anyways now.
> Looking further, then, it looks like in every non-sparc pte_index() is
> an actual numerical index, while on sparc it goes a step further to
> yield a pte_t *.
> As far as I can tell, the sparc incarnation of this is only used by
> the pte_offset_(kernel/map) macros.
> 
> So I think a possibly sane way to fix this would be:
> 1. Define pte_index() to be a numerical index, like the other architectures,
> 2. Define something like pte_entry() that uses pte_index(), and
> 3. Have pte_offset_(kernel/map) be defined as pte_entry() instead.
> 
> Then pte_index would be operating on just an address for all
> platforms, and the reverted patchset would work without any changes.
> 
> If this sounds acceptable, I can send a patch.
> 
> > > I have reverted these commits for today:
> > >
> > >   219ae14a9686 ("net-zerocopy-use-vm_insert_pages-for-tcp-rcv-zerocopy-fix")
> > >   cb912fdf96bf ("net-zerocopy: use vm_insert_pages() for tcp rcv zerocopy")
> > >   72c684430b94 ("add missing page_count() check to vm_insert_pages().")
> > >   dbd9553775f3 ("mm-add-vm_insert_pages-fix")
> > >   366142f0b000 ("mm/memory.c: add vm_insert_pages()")
> > >  
> >
> > In terms of fixing this; passing in an appropriate dir parameter is
> > not really a problem, but what is concerning that it seems messy to
> > have a per-platform ifdef to pass it either two arguments or one in
> > this case. But it seems like either that would be one way to fix it,
> > or having some arch method across all arches that takes two arguments
> > (and ignores one of them for most arches).
> >
> > Is there a general preference for the right way forward, in this case?

Has there been any progress with this?  I am still reverting the above
commits, so they are not getting any linux-next testing ...

-- 
Cheers,
Stephen Rothwell