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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 16 Feb 2020 22:45:35 -0800
From:   Arjun Roy <arjunroy@...gle.com>
To:     Stephen Rothwell <sfr@...b.auug.org.au>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: linux-next: build failure after merge of the akpm tree

On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <arjunroy@...gle.com> wrote:
>
> On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <sfr@...b.auug.org.au> 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