Hi all,
After merging the akpm-current tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:
In file included from include/linux/rcupdate.h:28,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/arm/kernel/asm-offsets.c:11:
include/linux/bottom_half.h: In function 'local_bh_disable':
include/linux/bottom_half.h:19:24: error: '_THIS_IP_' undeclared (first use in this function)
19 | __local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
| ^~~~~~~~~
include/linux/bottom_half.h:19:24: note: each undeclared identifier is reported only once for each function it appears in
include/linux/bottom_half.h: In function 'local_bh_enable':
include/linux/bottom_half.h:32:23: error: '_THIS_IP_' undeclared (first use in this function)
32 | __local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
| ^~~~~~~~~
Presumably caused by a commit in the series that starts with
dcaf7a5f413b ("kernel.h: drop unneeded <linux/kernel.h> inclusion from other headers")
I have applied the following patch for today (though there may be a
better solution).
From: Stephen Rothwell <[email protected]>
Date: Fri, 15 Oct 2021 19:58:46 +1100
Subject: [PATCH] bottom_half.h needs kernel.h
for _THIS_IP_ on arm at least
Signed-off-by: Stephen Rothwell <[email protected]>
---
include/linux/bottom_half.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index eed86eb0a1de..11d107d88d03 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_BH_H
#define _LINUX_BH_H
+#include <linux/kernel.h>
#include <linux/preempt.h>
#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_TRACE_IRQFLAGS)
--
2.33.0
--
Cheers,
Stephen Rothwell
+Cc: Rasmus
On Fri, Oct 15, 2021 at 08:29:08PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the akpm-current tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> In file included from include/linux/rcupdate.h:28,
> from include/linux/rculist.h:11,
> from include/linux/pid.h:5,
> from include/linux/sched.h:14,
> from arch/arm/kernel/asm-offsets.c:11:
> include/linux/bottom_half.h: In function 'local_bh_disable':
> include/linux/bottom_half.h:19:24: error: '_THIS_IP_' undeclared (first use in this function)
> 19 | __local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
> | ^~~~~~~~~
> include/linux/bottom_half.h:19:24: note: each undeclared identifier is reported only once for each function it appears in
> include/linux/bottom_half.h: In function 'local_bh_enable':
> include/linux/bottom_half.h:32:23: error: '_THIS_IP_' undeclared (first use in this function)
> 32 | __local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
> | ^~~~~~~~~
>
> Presumably caused by a commit in the series that starts with
>
> dcaf7a5f413b ("kernel.h: drop unneeded <linux/kernel.h> inclusion from other headers")
>
> I have applied the following patch for today (though there may be a
> better solution).
Thanks! As a quick fix looks good, but I think we need a separate header for
those _*_IP_ macros.
> From: Stephen Rothwell <[email protected]>
> Date: Fri, 15 Oct 2021 19:58:46 +1100
> Subject: [PATCH] bottom_half.h needs kernel.h
>
> for _THIS_IP_ on arm at least
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> include/linux/bottom_half.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
> index eed86eb0a1de..11d107d88d03 100644
> --- a/include/linux/bottom_half.h
> +++ b/include/linux/bottom_half.h
> @@ -2,6 +2,7 @@
> #ifndef _LINUX_BH_H
> #define _LINUX_BH_H
>
> +#include <linux/kernel.h>
> #include <linux/preempt.h>
>
> #if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_TRACE_IRQFLAGS)
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Fri, 15 Oct 2021 16:14:56 +0300 Andy Shevchenko <[email protected]> wrote:
>
> Thanks! As a quick fix looks good, but I think we need a separate header for
> those _*_IP_ macros.
Like this (on top of my previous fix - which I assume Andrew will
squash out of existence)?
From: Stephen Rothwell <[email protected]>
Date: Mon, 18 Oct 2021 13:27:54 +1100
Subject: [PATCH] kernel.h: split out instrcutions pointer accessors
botton_half.h needs _THIS_IP_ to be standalone, so split that and _RET_IP_
out from kernel.h into the new instruction_pointer.h. kernel.h directly
needs them, so include it there and replace the include of kernel.h with
this new file in bottom_half.h.
Signed-off-by: Stephen Rothwell <[email protected]>
---
include/linux/bottom_half.h | 2 +-
include/linux/instruction_pointer.h | 8 ++++++++
include/linux/kernel.h | 4 +---
3 files changed, 10 insertions(+), 4 deletions(-)
create mode 100644 include/linux/instruction_pointer.h
diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index 11d107d88d03..fc53e0ad56d9 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -2,7 +2,7 @@
#ifndef _LINUX_BH_H
#define _LINUX_BH_H
-#include <linux/kernel.h>
+#include <linux/instruction_pointer.h>
#include <linux/preempt.h>
#if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_TRACE_IRQFLAGS)
diff --git a/include/linux/instruction_pointer.h b/include/linux/instruction_pointer.h
new file mode 100644
index 000000000000..19e979425bda
--- /dev/null
+++ b/include/linux/instruction_pointer.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_INSTRUCTION_POINTER_H
+#define _LINUX_INSTRUCTION_POINTER_H
+
+#define _RET_IP_ (unsigned long)__builtin_return_address(0)
+#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
+
+#enfif /* _LINUX_INSTRUCTION_POINTER_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 973c61ff2ef9..be84ab369650 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -20,6 +20,7 @@
#include <linux/printk.h>
#include <linux/build_bug.h>
#include <linux/static_call_types.h>
+#include <linux/instruction_pointer.h>
#include <asm/byteorder.h>
#include <uapi/linux/kernel.h>
@@ -53,9 +54,6 @@
} \
)
-#define _RET_IP_ (unsigned long)__builtin_return_address(0)
-#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
-
/**
* upper_32_bits - return bits 32-63 of a number
* @n: the number we're accessing
--
2.33.0
There are, presumably, other places this new file can be included ...
--
Cheers,
Stephen Rothwell
On Mon, Oct 18, 2021 at 6:49 AM Stephen Rothwell <[email protected]> wrote:
> On Fri, 15 Oct 2021 16:14:56 +0300 Andy Shevchenko <[email protected]> wrote:
> >
> > Thanks! As a quick fix looks good, but I think we need a separate header for
> > those _*_IP_ macros.
>
> Like this (on top of my previous fix - which I assume Andrew will
> squash out of existence)?
Yep, thanks!
I thought that it makes sense to have STACK_MAGIC also in this header. Thoughts?
One spelling correction below.
> From: Stephen Rothwell <[email protected]>
> Date: Mon, 18 Oct 2021 13:27:54 +1100
> Subject: [PATCH] kernel.h: split out instrcutions pointer accessors
instructions
> botton_half.h needs _THIS_IP_ to be standalone, so split that and _RET_IP_
> out from kernel.h into the new instruction_pointer.h. kernel.h directly
> needs them, so include it there and replace the include of kernel.h with
> this new file in bottom_half.h.
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> include/linux/bottom_half.h | 2 +-
> include/linux/instruction_pointer.h | 8 ++++++++
> include/linux/kernel.h | 4 +---
> 3 files changed, 10 insertions(+), 4 deletions(-)
> create mode 100644 include/linux/instruction_pointer.h
>
> diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
> index 11d107d88d03..fc53e0ad56d9 100644
> --- a/include/linux/bottom_half.h
> +++ b/include/linux/bottom_half.h
> @@ -2,7 +2,7 @@
> #ifndef _LINUX_BH_H
> #define _LINUX_BH_H
>
> -#include <linux/kernel.h>
> +#include <linux/instruction_pointer.h>
> #include <linux/preempt.h>
>
> #if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_TRACE_IRQFLAGS)
> diff --git a/include/linux/instruction_pointer.h b/include/linux/instruction_pointer.h
> new file mode 100644
> index 000000000000..19e979425bda
> --- /dev/null
> +++ b/include/linux/instruction_pointer.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_INSTRUCTION_POINTER_H
> +#define _LINUX_INSTRUCTION_POINTER_H
> +
> +#define _RET_IP_ (unsigned long)__builtin_return_address(0)
> +#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
> +
> +#enfif /* _LINUX_INSTRUCTION_POINTER_H */
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 973c61ff2ef9..be84ab369650 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -20,6 +20,7 @@
> #include <linux/printk.h>
> #include <linux/build_bug.h>
> #include <linux/static_call_types.h>
> +#include <linux/instruction_pointer.h>
> #include <asm/byteorder.h>
>
> #include <uapi/linux/kernel.h>
> @@ -53,9 +54,6 @@
> } \
> )
>
> -#define _RET_IP_ (unsigned long)__builtin_return_address(0)
> -#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
> -
> /**
> * upper_32_bits - return bits 32-63 of a number
> * @n: the number we're accessing
> --
> 2.33.0
>
> There are, presumably, other places this new file can be included ...
>
> --
> Cheers,
> Stephen Rothwell
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Mon, 18 Oct 2021 11:01:18 +0300 Andy Shevchenko <[email protected]> wrote:
>
> On Mon, Oct 18, 2021 at 6:49 AM Stephen Rothwell <[email protected]> wrote:
> > On Fri, 15 Oct 2021 16:14:56 +0300 Andy Shevchenko <[email protected]> wrote:
> > >
> > > Thanks! As a quick fix looks good, but I think we need a separate header for
> > > those _*_IP_ macros.
> >
> > Like this (on top of my previous fix - which I assume Andrew will
> > squash out of existence)?
>
> Yep, thanks!
> I thought that it makes sense to have STACK_MAGIC also in this header. Thoughts?
You might want to think of a different name for the header file then.
>
> One spelling correction below.
>
> > From: Stephen Rothwell <[email protected]>
> > Date: Mon, 18 Oct 2021 13:27:54 +1100
> > Subject: [PATCH] kernel.h: split out instrcutions pointer accessors
>
> instructions
instruction ? :-)
--
Cheers,
Stephen Rothwell
On 18/10/2021 10.45, Stephen Rothwell wrote:
> Hi Andy,
>
> On Mon, 18 Oct 2021 11:01:18 +0300 Andy Shevchenko <[email protected]> wrote:
>>
>> On Mon, Oct 18, 2021 at 6:49 AM Stephen Rothwell <[email protected]> wrote:
>>> On Fri, 15 Oct 2021 16:14:56 +0300 Andy Shevchenko <[email protected]> wrote:
>>>>
>>>> Thanks! As a quick fix looks good, but I think we need a separate header for
>>>> those _*_IP_ macros.
>>>
>>> Like this (on top of my previous fix - which I assume Andrew will
>>> squash out of existence)?
>>
>> Yep, thanks!
>> I thought that it makes sense to have STACK_MAGIC also in this header. Thoughts?
>
> You might want to think of a different name for the header file then.
Eh, it seems more reasonable to leave it in kernel.h, then figure out
how to get rid of it completely. AFAICT it's only used in one single
place under arch/ (and I can't figure out how that magic value is
supposed to get there in the first place... that arch was thrown out in
2013 and resurrected in 2015, but that particular line doesn't make
sense), and then in some i915 code which might as well define their own
cookie.
Rasmus
On Mon, Oct 18, 2021 at 12:07 PM Rasmus Villemoes
<[email protected]> wrote:
> On 18/10/2021 10.45, Stephen Rothwell wrote:
> > On Mon, 18 Oct 2021 11:01:18 +0300 Andy Shevchenko <[email protected]> wrote:
> >> On Mon, Oct 18, 2021 at 6:49 AM Stephen Rothwell <[email protected]> wrote:
...
> >> I thought that it makes sense to have STACK_MAGIC also in this header. Thoughts?
> >
> > You might want to think of a different name for the header file then.
>
> Eh, it seems more reasonable to leave it in kernel.h, then figure out
> how to get rid of it completely. AFAICT it's only used in one single
> place under arch/ (and I can't figure out how that magic value is
> supposed to get there in the first place... that arch was thrown out in
> 2013 and resurrected in 2015, but that particular line doesn't make
> sense), and then in some i915 code which might as well define their own
> cookie.
It's used in two places and probably we may just move it to these two
users, I don't believe will we ever have i915 together with h8300.
--
With Best Regards,
Andy Shevchenko
On Mon, Oct 18, 2021 at 01:35:38PM +1100, Stephen Rothwell wrote:
> On Fri, 15 Oct 2021 16:14:56 +0300 Andy Shevchenko <[email protected]> wrote:
...
> Like this (on top of my previous fix - which I assume Andrew will
> squash out of existence)?
Andrew, can you incorporate this one, please?
> From: Stephen Rothwell <[email protected]>
> Date: Mon, 18 Oct 2021 13:27:54 +1100
> Subject: [PATCH] kernel.h: split out instrcutions pointer accessors
>
> botton_half.h needs _THIS_IP_ to be standalone, so split that and _RET_IP_
> out from kernel.h into the new instruction_pointer.h. kernel.h directly
> needs them, so include it there and replace the include of kernel.h with
> this new file in bottom_half.h.
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> include/linux/bottom_half.h | 2 +-
> include/linux/instruction_pointer.h | 8 ++++++++
> include/linux/kernel.h | 4 +---
> 3 files changed, 10 insertions(+), 4 deletions(-)
> create mode 100644 include/linux/instruction_pointer.h
>
> diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
> index 11d107d88d03..fc53e0ad56d9 100644
> --- a/include/linux/bottom_half.h
> +++ b/include/linux/bottom_half.h
> @@ -2,7 +2,7 @@
> #ifndef _LINUX_BH_H
> #define _LINUX_BH_H
>
> -#include <linux/kernel.h>
> +#include <linux/instruction_pointer.h>
> #include <linux/preempt.h>
>
> #if defined(CONFIG_PREEMPT_RT) || defined(CONFIG_TRACE_IRQFLAGS)
> diff --git a/include/linux/instruction_pointer.h b/include/linux/instruction_pointer.h
> new file mode 100644
> index 000000000000..19e979425bda
> --- /dev/null
> +++ b/include/linux/instruction_pointer.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_INSTRUCTION_POINTER_H
> +#define _LINUX_INSTRUCTION_POINTER_H
> +
> +#define _RET_IP_ (unsigned long)__builtin_return_address(0)
> +#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
> +
> +#enfif /* _LINUX_INSTRUCTION_POINTER_H */
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 973c61ff2ef9..be84ab369650 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -20,6 +20,7 @@
> #include <linux/printk.h>
> #include <linux/build_bug.h>
> #include <linux/static_call_types.h>
> +#include <linux/instruction_pointer.h>
> #include <asm/byteorder.h>
>
> #include <uapi/linux/kernel.h>
> @@ -53,9 +54,6 @@
> } \
> )
>
> -#define _RET_IP_ (unsigned long)__builtin_return_address(0)
> -#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; })
> -
> /**
> * upper_32_bits - return bits 32-63 of a number
> * @n: the number we're accessing
> --
> 2.33.0
--
With Best Regards,
Andy Shevchenko