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

[Arjun Roy]

On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <arjunroy@xxxxxxxxxx> wrote:
>
> On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
> >
> > Hi all,
> >
> > 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.

Thanks!



> > 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?
>
> Thanks,
> -Arjun
>
> > --
> > Cheers,
> > Stephen Rothwell