2015-12-14 16:08:53

by Julien Grall

[permalink] [raw]
Subject: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:

In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
from linux/arch/arm64/include/asm/xen/page.h:1,
from linux/include/xen/page.h:28,
from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
#define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
^
linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
VM_WARN_ONCE(!pte_young(pte),
^

This has been introduced by commit 82d340081b6f71237373d1452e3573a5a122794c
"arm64: Improve error reporting on set_pte_at() checks". This is because
mmdebug.h relies on the includer to properly include the dependencies.

Signed-off-by: Julien Grall <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>

I was tempted to add the missing include in linux/mmdebug.h but I'm
not sure about the policy for headers inclusion in Linux.

Also, I'm not sure why the compiler error only happen with CONFIG_XEN=y.
---
arch/arm64/include/asm/pgtable.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 63f52b5..0dd96f3 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -57,6 +57,7 @@

#ifndef __ASSEMBLY__

+#include <linux/bug.h>
#include <linux/mmdebug.h>

extern void __pte_error(const char *file, int line, unsigned long val);
--
2.1.4


2015-12-14 16:22:47

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

On Mon, Dec 14, 2015 at 04:07:24PM +0000, Julien Grall wrote:
> Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:
>
> In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
> from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
> from linux/arch/arm64/include/asm/xen/page.h:1,
> from linux/include/xen/page.h:28,
> from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
> linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
> linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
> #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
> ^
> linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
> VM_WARN_ONCE(!pte_young(pte),
> ^
>
> This has been introduced by commit 82d340081b6f71237373d1452e3573a5a122794c
> "arm64: Improve error reporting on set_pte_at() checks". This is because
> mmdebug.h relies on the includer to properly include the dependencies.
>
> Signed-off-by: Julien Grall <[email protected]>
>
> ---
> Cc: Stefano Stabellini <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
>
> I was tempted to add the missing include in linux/mmdebug.h but I'm
> not sure about the policy for headers inclusion in Linux.

Normally, I would say that mmdebug.h should include whichever headers it
needs but for a quicker fix, I'm fine with including linux/bug.h (and
probably removing the asm/bug.h include in asm/pgtable.h).

--
Catalin

2015-12-14 16:28:16

by Julien Grall

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

Hi Catalin,

On 14/12/15 16:22, Catalin Marinas wrote:
> On Mon, Dec 14, 2015 at 04:07:24PM +0000, Julien Grall wrote:
>> Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:
>>
>> In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
>> from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
>> from linux/arch/arm64/include/asm/xen/page.h:1,
>> from linux/include/xen/page.h:28,
>> from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
>> linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
>> linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
>> #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
>> ^
>> linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
>> VM_WARN_ONCE(!pte_young(pte),
>> ^
>>
>> This has been introduced by commit 82d340081b6f71237373d1452e3573a5a122794c
>> "arm64: Improve error reporting on set_pte_at() checks". This is because
>> mmdebug.h relies on the includer to properly include the dependencies.
>>
>> Signed-off-by: Julien Grall <[email protected]>
>>
>> ---
>> Cc: Stefano Stabellini <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>>
>> I was tempted to add the missing include in linux/mmdebug.h but I'm
>> not sure about the policy for headers inclusion in Linux.
>
> Normally, I would say that mmdebug.h should include whichever headers it
> needs but for a quicker fix, I'm fine with including linux/bug.h (and
> probably removing the asm/bug.h include in asm/pgtable.h).

The linux/bug.h will only be included when __ASSEMBLY__ is not defined
which asm/bug.h is included even for assembly file.

Note that it's not possible to include linux/bug.h for assembly file
because the headers is not correctly protected.

As I don't know if asm/bug.h is useful in pgtable.h, I've decided to
keep it.

Regards,

--
Julien Grall

2015-12-14 16:38:46

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

On Mon, Dec 14, 2015 at 04:26:48PM +0000, Julien Grall wrote:
> On 14/12/15 16:22, Catalin Marinas wrote:
> > On Mon, Dec 14, 2015 at 04:07:24PM +0000, Julien Grall wrote:
> >> Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:
> >>
> >> In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
> >> from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
> >> from linux/arch/arm64/include/asm/xen/page.h:1,
> >> from linux/include/xen/page.h:28,
> >> from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
> >> linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
> >> linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
> >> #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
> >> ^
> >> linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
> >> VM_WARN_ONCE(!pte_young(pte),
> >> ^
> >>
> >> This has been introduced by commit 82d340081b6f71237373d1452e3573a5a122794c
> >> "arm64: Improve error reporting on set_pte_at() checks". This is because
> >> mmdebug.h relies on the includer to properly include the dependencies.
> >>
> >> Signed-off-by: Julien Grall <[email protected]>
> >>
> >> ---
> >> Cc: Stefano Stabellini <[email protected]>
> >> Cc: Catalin Marinas <[email protected]>
> >> Cc: Will Deacon <[email protected]>
> >>
> >> I was tempted to add the missing include in linux/mmdebug.h but I'm
> >> not sure about the policy for headers inclusion in Linux.
> >
> > Normally, I would say that mmdebug.h should include whichever headers it
> > needs but for a quicker fix, I'm fine with including linux/bug.h (and
> > probably removing the asm/bug.h include in asm/pgtable.h).
>
> The linux/bug.h will only be included when __ASSEMBLY__ is not defined
> which asm/bug.h is included even for assembly file.
>
> Note that it's not possible to include linux/bug.h for assembly file
> because the headers is not correctly protected.
>
> As I don't know if asm/bug.h is useful in pgtable.h, I've decided to
> keep it.

I don't know why it's there either. Anyway, I'll apply your patch and
try to remove asm/bug.h as well. If the build fails, I'll add it back.

--
Catalin

2015-12-14 16:48:04

by James Morse

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

On 14/12/15 16:38, Catalin Marinas wrote:
> On Mon, Dec 14, 2015 at 04:26:48PM +0000, Julien Grall wrote:
>> On 14/12/15 16:22, Catalin Marinas wrote:
>>> On Mon, Dec 14, 2015 at 04:07:24PM +0000, Julien Grall wrote:
>>>> Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:
>>>>
>>>> In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
>>>> from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
>>>> from linux/arch/arm64/include/asm/xen/page.h:1,
>>>> from linux/include/xen/page.h:28,
>>>> from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
>>>> linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
>>>> linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
>>>> #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
>>>> ^
>>>> linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
>>>> VM_WARN_ONCE(!pte_young(pte),

I got bitten by this this morning, and had a patch to mmdebug.h I just
sent to the linux-mm list... sorry I should have copied everyone on this
thread (you don't get enough email already!).


Thanks,

James

2015-12-14 17:07:33

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

On Mon, Dec 14, 2015 at 04:47:07PM +0000, James Morse wrote:
> On 14/12/15 16:38, Catalin Marinas wrote:
> > On Mon, Dec 14, 2015 at 04:26:48PM +0000, Julien Grall wrote:
> >> On 14/12/15 16:22, Catalin Marinas wrote:
> >>> On Mon, Dec 14, 2015 at 04:07:24PM +0000, Julien Grall wrote:
> >>>> Linux 4.4-rc5 doesn't build for arm64 with CONFIG_XEN=y enabled:
> >>>>
> >>>> In file included from linux/arch/arm64/include/asm/pgtable.h:60:0,
> >>>> from linux/arch/arm64/include/../../arm/include/asm/xen/page.h:5,
> >>>> from linux/arch/arm64/include/asm/xen/page.h:1,
> >>>> from linux/include/xen/page.h:28,
> >>>> from linux/arch/arm64/xen/../../arm/xen/grant-table.c:33:
> >>>> linux/arch/arm64/include/asm/pgtable.h: In function 'set_pte_at':
> >>>> linux/include/linux/mmdebug.h:49:39: error: implicit declaration of function 'BUILD_BUG_ON_INVALID' [-Werror=implicit-function-declaration]
> >>>> #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
> >>>> ^
> >>>> linux/arch/arm64/include/asm/pgtable.h:281:3: note: in expansion of macro 'VM_WARN_ONCE'
> >>>> VM_WARN_ONCE(!pte_young(pte),
>
> I got bitten by this this morning, and had a patch to mmdebug.h I just
> sent to the linux-mm list... sorry I should have copied everyone on this
> thread (you don't get enough email already!).

If they take the mmdebug.h patch, it's even better. Please let us know
how that goes.

Thanks.

--
Catalin

2015-12-15 10:19:13

by James Morse

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add missing linux/bug.h include in asm/pgtable.h

On 14/12/15 17:07, Catalin Marinas wrote:
> If they take the mmdebug.h patch, it's even better. Please let us know
> how that goes.


Andrew Morton's Robot wrote:
> The patch titled
> Subject: include/linux/mmdebug.h: should include linux/bug.h
> has been added to the -mm tree. Its filename is
> include-linux-mmdebugh-should-include-linux-bugh.patch
>
> This patch should soon appear at
> http://ozlabs.org/~akpm/mmots/broken-out/include-linux-mmdebugh-should-include-linux-bugh.patch
> and later at
> http://ozlabs.org/~akpm/mmotm/broken-out/include-linux-mmdebugh-should-include-linux-bugh.patch
>
> Before you just go and hit "reply", please:
> a) Consider who else should be cc'ed
> b) Prefer to cc a suitable mailing list as well
> c) Ideally: find the original patch on the mailing list and do a
> reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>
> The -mm tree is included into linux-next and is updated
> there every 3-4 working days




Thanks,

James