Hi all,
On Sun, 16 Feb 2020 22:45:35 -0800 Arjun Roy <arjunroy@xxxxxxxxxx> wrote:
>
> 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:
> > >
> > > 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
Attachment:
pgpMrKkMlMSgE.pgp
Description: OpenPGP digital signature
