diff mbox

mm/memblock: add missing include <linux/bootmem.h>

Message ID 20180615121716.37fb93385825b0b2f59240cc@linux-foundation.org (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Morton June 15, 2018, 7:17 p.m. UTC 
On Fri, 15 Jun 2018 11:59:10 -0700 Tony Luck <tony.luck@gmail.com> wrote:

> On Wed, Jun 6, 2018 at 12:41 PM, Mathieu Malaterre <malat@debian.org> wrote:
> > Commit 26f09e9b3a06 ("mm/memblock: add memblock memory allocation apis")
> > introduced two new function definitions:
> >   ‘memblock_virt_alloc_try_nid_nopanic’
> > and
> >   ‘memblock_virt_alloc_try_nid’.
> > Commit ea1f5f3712af ("mm: define memblock_virt_alloc_try_nid_raw")
> > introduced the following function definition:
> >   ‘memblock_virt_alloc_try_nid_raw’
> >
> > This commit adds an includeof header file <linux/bootmem.h> to provide the
> > missing function prototypes. Silence the following gcc warning (W=1):
> >
> >   mm/memblock.c:1334:15: warning: no previous prototype for ‘memblock_virt_alloc_try_nid_raw’ [-Wmissing-prototypes]
> >   mm/memblock.c:1371:15: warning: no previous prototype for ‘memblock_virt_alloc_try_nid_nopanic’ [-Wmissing-prototypes]
> >   mm/memblock.c:1407:15: warning: no previous prototype for ‘memblock_virt_alloc_try_nid’ [-Wmissing-prototypes]
> >
> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
> 
> Sadly that breaks ia64 build:
> 
>   CC      mm/memblock.o
> mm/memblock.c:1340: error: redefinition of ‘memblock_virt_alloc_try_nid_raw’
> ./include/linux/bootmem.h:335: error: previous definition of
> ‘memblock_virt_alloc_try_nid_raw’ was here
> mm/memblock.c:1377: error: redefinition of ‘memblock_virt_alloc_try_nid_nopanic’
> ./include/linux/bootmem.h:343: error: previous definition of
> ‘memblock_virt_alloc_try_nid_nopanic’ was here
> mm/memblock.c:1413: error: redefinition of ‘memblock_virt_alloc_try_nid’
> ./include/linux/bootmem.h:327: error: previous definition of
> ‘memblock_virt_alloc_try_nid’ was here
> make[1]: *** [mm/memblock.o] Error 1
> make: *** [mm/memblock.o] Error 2

Huh.  How did that ever work.  I guess it's either this:



and I'm not sure which.  I think I'll just revert $subject for now.

Comments

Tony Luck June 15, 2018, 10:26 p.m. UTC | #1 
On Fri, Jun 15, 2018 at 12:17 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:

> Huh.  How did that ever work.  I guess it's either this:
>
> --- a/mm/Makefile~a
> +++ a/mm/Makefile
> @@ -45,6 +45,7 @@ obj-y += init-mm.o
>
>  ifdef CONFIG_NO_BOOTMEM
>         obj-y           += nobootmem.o
> +       obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
>  else
>         obj-y           += bootmem.o
>  endif
> @@ -53,7 +54,6 @@ obj-$(CONFIG_ADVISE_SYSCALLS) += fadvise
>  ifdef CONFIG_MMU
>         obj-$(CONFIG_ADVISE_SYSCALLS)   += madvise.o
>  endif
> -obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
>
>  obj-$(CONFIG_SWAP)     += page_io.o swap_state.o swapfile.o swap_slots.o
>  obj-$(CONFIG_FRONTSWAP)        += frontswap.o

That option gave me a boatload of undefined symbols.

> or this:
>
> --- a/include/linux/bootmem.h~a
> +++ a/include/linux/bootmem.h
> @@ -154,7 +154,7 @@ extern void *__alloc_bootmem_low_node(pg
>         __alloc_bootmem_low_node(pgdat, x, PAGE_SIZE, 0)
>
>
> -#if defined(CONFIG_HAVE_MEMBLOCK) && defined(CONFIG_NO_BOOTMEM)
> +#if defined(CONFIG_HAVE_MEMBLOCK)
>
>  /* FIXME: use MEMBLOCK_ALLOC_* variants here */
>  #define BOOTMEM_ALLOC_ACCESSIBLE       0

That compiles cleanly, but didn't boot:

 [<a000000100029910>] ia64_fault+0xf0/0xe00
                                sp=e0000004fb37f8a0 bsp=e0000004fb371438
 [<a00000010000c920>] ia64_leave_kernel+0x0/0x270
                                sp=e0000004fb37fba0 bsp=e0000004fb371438
hid-generic 0003:0624:0200.0001: input: USB HID v1.10 Mouse [Avocent
USB_AMIQ] on usb-0000:00:1d.0-2/input1
 [<a00000010020b100>] pcpu_find_block_fit+0x20/0x300
                                sp=e0000004fb37fd70 bsp=e0000004fb3713a8
 [<a00000010020ee70>] pcpu_alloc+0x630/0xc40
                                sp=e0000004fb37fd90 bsp=e0000004fb371308
input: Avocent USB_AMIQ as
/devices/pci0000:00/0000:00:1d.0/usb4/4-2/4-2:1.0/0003:0624:0200.0002/input/input3
 [<a00000010020f520>] __alloc_percpu+0x40/0x60
                                sp=e0000004fb37fda0 bsp=e0000004fb3712e0
 [<a0000001002fb4c0>] alloc_vfsmnt+0x1c0/0x4e0
                                sp=e0000004fb37fda0 bsp=e0000004fb371280
 [<a000000100303d10>] vfs_kern_mount+0x30/0x2a0
                                sp=e0000004fb37fdf0 bsp=e0000004fb371238


> and I'm not sure which.  I think I'll just revert $subject for now.

Reverting is a good short term fix.

-Tony
Paul Burton June 21, 2018, 6:06 p.m. UTC | #2 
Hi Andrew & Stephen,

On Fri, Jun 15, 2018 at 12:17:16PM -0700, Andrew Morton wrote:
> > Sadly that breaks ia64 build:
> > 
> >   CC      mm/memblock.o
> > mm/memblock.c:1340: error: redefinition of ‘memblock_virt_alloc_try_nid_raw’
> > ./include/linux/bootmem.h:335: error: previous definition of
> > ‘memblock_virt_alloc_try_nid_raw’ was here
> > mm/memblock.c:1377: error: redefinition of ‘memblock_virt_alloc_try_nid_nopanic’
> > ./include/linux/bootmem.h:343: error: previous definition of
> > ‘memblock_virt_alloc_try_nid_nopanic’ was here
> > mm/memblock.c:1413: error: redefinition of ‘memblock_virt_alloc_try_nid’
> > ./include/linux/bootmem.h:327: error: previous definition of
> > ‘memblock_virt_alloc_try_nid’ was here
> > make[1]: *** [mm/memblock.o] Error 1
> > make: *** [mm/memblock.o] Error 2
> 
> Huh.  How did that ever work.  I guess it's either this:
<snip>
> and I'm not sure which.  I think I'll just revert $subject for now.

This is fine now in master after Andrew's revert, but the problematic
patch is still being picked up in linux-next somehow. This breaks MIPS
builds from linux-next, and presumably the ia64 build too.

I'm not sure I understand how it's picked up - next-20180621 appears to
based atop 1abd8a8f39cd:

  $ git show next-20180621:Next/SHA1s | grep -E '^origin\s'
  origin          1abd8a8f39cd9a2925149000056494523c85643a

There we have the Andrew's revert:

  $ git log --pretty=oneline -n5 1abd8a8f39cd mm/memblock.c
  6cc22dc08a247b7b4a173e4561e39705a557d300 revert "mm/memblock: add missing include <linux/bootmem.h>"
  0825a6f98689d847ab8058c51b3a55f0abcc6563 mm: use octal not symbolic permissions
  69b5086b12cda645d95f00575c25f1dfd1e929ad mm/memblock: add missing include <linux/bootmem.h>
  25cf23d7a95716fc6eb165208b5eb2e3b2e86f82 mm/memblock: print memblock_remove
  1c4bc43ddfd52cbe5a08bb86ae636f55d2799424 mm/memblock: introduce PHYS_ADDR_MAX

Yet the revert doesn't show up at all in next-20180621..?

  $ git log --pretty=oneline -n5 next-20180621 mm/memblock.c
  a95f41a659344e221e8ad39e8fbba2e0f419c096 mm: use octal not symbolic permissions
  0b558dea04a405800505c7f56eb1638ae761b5d4 mm/memblock: add missing include <linux/bootmem.h>
  25cf23d7a95716fc6eb165208b5eb2e3b2e86f82 mm/memblock: print memblock_remove
  1c4bc43ddfd52cbe5a08bb86ae636f55d2799424 mm/memblock: introduce PHYS_ADDR_MAX
  49a695ba723224875df50e327bd7b0b65dd9a56b Merge tag 'powerpc-4.17-1' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux

I was expecting to see the original commit, then the revert, then
perhaps a re-application of it but instead it looks like the commits
from master are missing entirely after 25cf23d7a957 ("mm/memblock: print
memblock_remove"). Maybe I'm missing something about the way the merges
for linux-next are done..?

In any case, could we get the problematic patch removed from linux-next?

Thanks,
    Paul
Stephen Rothwell June 21, 2018, 11:34 p.m. UTC | #3 
Hi Paul,

On Thu, 21 Jun 2018 11:06:38 -0700 Paul Burton <paul.burton@mips.com> wrote:
>
> I was expecting to see the original commit, then the revert, then
> perhaps a re-application of it but instead it looks like the commits
> from master are missing entirely after 25cf23d7a957 ("mm/memblock: print
> memblock_remove"). Maybe I'm missing something about the way the merges
> for linux-next are done..?

Andrew produces his mmotm quilt series and exports it to ozlabs.org
from where I fetch it and create the akpm-current and akpm branches in
linux-next (and merge them, obviously :-)).  The mmotm quilt series has
not changed since Jun 15, I assume Andrew is still finalising it.

> In any case, could we get the problematic patch removed from linux-next?

I have removed it from my copy of mmotm for today.
diff mbox

Patch

--- a/mm/Makefile~a
+++ a/mm/Makefile
@@ -45,6 +45,7 @@  obj-y += init-mm.o
 
 ifdef CONFIG_NO_BOOTMEM
 	obj-y		+= nobootmem.o
+	obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
 else
 	obj-y		+= bootmem.o
 endif
@@ -53,7 +54,6 @@  obj-$(CONFIG_ADVISE_SYSCALLS)	+= fadvise
 ifdef CONFIG_MMU
 	obj-$(CONFIG_ADVISE_SYSCALLS)	+= madvise.o
 endif
-obj-$(CONFIG_HAVE_MEMBLOCK) += memblock.o
 
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o swap_slots.o
 obj-$(CONFIG_FRONTSWAP)	+= frontswap.o


or this:

--- a/include/linux/bootmem.h~a
+++ a/include/linux/bootmem.h
@@ -154,7 +154,7 @@  extern void *__alloc_bootmem_low_node(pg
 	__alloc_bootmem_low_node(pgdat, x, PAGE_SIZE, 0)
 
 
-#if defined(CONFIG_HAVE_MEMBLOCK) && defined(CONFIG_NO_BOOTMEM)
+#if defined(CONFIG_HAVE_MEMBLOCK)
 
 /* FIXME: use MEMBLOCK_ALLOC_* variants here */
 #define BOOTMEM_ALLOC_ACCESSIBLE	0