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.
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()")
--
Cheers,
Stephen Rothwell
On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <[email protected]> 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.
> 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
On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <[email protected]> wrote:
>
> On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <[email protected]> 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
Hi all,
On Sun, 16 Feb 2020 22:45:35 -0800 Arjun Roy <[email protected]> wrote:
>
> On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <[email protected]> wrote:
> >
> > On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <[email protected]> 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
On Thu, Feb 20, 2020 at 3:18 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> On Sun, 16 Feb 2020 22:45:35 -0800 Arjun Roy <[email protected]> wrote:
> >
> > On Sun, Feb 16, 2020 at 8:12 PM Arjun Roy <[email protected]> wrote:
> > >
> > > On Sun, Feb 16, 2020 at 7:57 PM Stephen Rothwell <[email protected]> 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 ...
>
I have a possible solution in mind, but it would involve a slight
change in the SPARC macro (to be more inline with the semantics of the
other platforms).
If you're open to such a change, I can send it out.
Thanks,
-Arjun
> --
> Cheers,
> Stephen Rothwell
Hi Arjun,
On Thu, 20 Feb 2020 15:22:04 -0800 Arjun Roy <[email protected]> wrote:
>
> I have a possible solution in mind, but it would involve a slight
> change in the SPARC macro (to be more inline with the semantics of the
> other platforms).
> If you're open to such a change, I can send it out.
Its not up to me :-)
If it is not too much work, I would say, do the patch, test it as you
can, then send it out cc'ing the Sparc maintainer (DaveM cc'd) and see
what happens.
--
Cheers,
Stephen Rothwell
On Thu, Feb 20, 2020 at 3:43 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Arjun,
>
> On Thu, 20 Feb 2020 15:22:04 -0800 Arjun Roy <[email protected]> wrote:
> >
> > I have a possible solution in mind, but it would involve a slight
> > change in the SPARC macro (to be more inline with the semantics of the
> > other platforms).
> > If you're open to such a change, I can send it out.
>
> Its not up to me :-)
>
> If it is not too much work, I would say, do the patch, test it as you
> can, then send it out cc'ing the Sparc maintainer (DaveM cc'd) and see
> what happens.
>
Certainly, I will do so.
Thanks,
-Arjun
> --
> Cheers,
> Stephen Rothwell
Hi Arjun,
On Thu, 20 Feb 2020 15:44:28 -0800 Arjun Roy <[email protected]> wrote:
>
> On Thu, Feb 20, 2020 at 3:43 PM Stephen Rothwell <[email protected]> wrote:
> >
> > On Thu, 20 Feb 2020 15:22:04 -0800 Arjun Roy <[email protected]> wrote:
> > >
> > > I have a possible solution in mind, but it would involve a slight
> > > change in the SPARC macro (to be more inline with the semantics of the
> > > other platforms).
> > > If you're open to such a change, I can send it out.
> >
> > Its not up to me :-)
> >
> > If it is not too much work, I would say, do the patch, test it as you
> > can, then send it out cc'ing the Sparc maintainer (DaveM cc'd) and see
> > what happens.
>
> Certainly, I will do so.
Just one thing: its worth while making it clear to DaveM that you just
want his Ack (rather than for him to take the patch into the Sparc
tree) so that the patch can go into Andrew's tree along with the rest
of the series.
--
Cheers,
Stephen Rothwell
On Thu, Feb 20, 2020 at 3:59 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Arjun,
>
> On Thu, 20 Feb 2020 15:44:28 -0800 Arjun Roy <[email protected]> wrote:
> >
> > On Thu, Feb 20, 2020 at 3:43 PM Stephen Rothwell <[email protected]> wrote:
> > >
> > > On Thu, 20 Feb 2020 15:22:04 -0800 Arjun Roy <[email protected]> wrote:
> > > >
> > > > I have a possible solution in mind, but it would involve a slight
> > > > change in the SPARC macro (to be more inline with the semantics of the
> > > > other platforms).
> > > > If you're open to such a change, I can send it out.
> > >
> > > Its not up to me :-)
> > >
> > > If it is not too much work, I would say, do the patch, test it as you
> > > can, then send it out cc'ing the Sparc maintainer (DaveM cc'd) and see
> > > what happens.
> >
> > Certainly, I will do so.
>
> Just one thing: its worth while making it clear to DaveM that you just
> want his Ack (rather than for him to take the patch into the Sparc
> tree) so that the patch can go into Andrew's tree along with the rest
> of the series.
>
For now since the earlier patches remain in the mm tree (as far as I
can tell here https://github.com/hnaz/linux-mm/tree/master) I have
just sent a fixup patch for the sparc compilation issue to mm-next. I
also indicated the ack request to DaveM.
Thanks,
-Arjun
> --
> Cheers,
> Stephen Rothwell
Hi all,
On Mon, 17 Feb 2020 14:57:11 +1100 Stephen Rothwell <[email protected]> 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.
>
> 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()")
This failure is now in the akpm tree (it was previously in the
skpm-current tree) but I am still reverting the same patches (though
they are now slightly different).
--
Cheers,
Stephen Rothwell
On Tue, Feb 25, 2020 at 8:02 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> On Mon, 17 Feb 2020 14:57:11 +1100 Stephen Rothwell <[email protected]> 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.
> >
> > 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()")
>
> This failure is now in the akpm tree (it was previously in the
> skpm-current tree) but I am still reverting the same patches (though
> they are now slightly different).
>
Ack, and they will continue to fail till my fixup patch for sparc64 is
merged (it's already out for review).
-Arjun
> --
> Cheers,
> Stephen Rothwell