linux-next: build failure after merge of the printk tree

linux-next: build failure after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Hi all,

After merging the printk tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from include/linux/printk.h:10,
                 from include/linux/kernel.h:15,
                 from include/linux/list.h:9,
                 from include/linux/lockdep.h:43,
                 from include/linux/spinlock_types.h:18,
                 from include/linux/genalloc.h:32,
                 from drivers/soc/fsl/qe/qe_common.c:16:
include/linux/ratelimit_types.h:16:2: error: unknown type name 'raw_spinlock_t'
   16 |  raw_spinlock_t lock;  /* protect the state */
      |  ^~~~~~~~~~~~~~
In file included from include/linux/wait.h:9,
                 from include/linux/pid.h:6,
                 from include/linux/sched.h:14,
                 from include/linux/ratelimit.h:6,
                 from include/linux/dev_printk.h:16,
                 from include/linux/device.h:15,
                 from include/linux/node.h:18,
                 from include/linux/cpu.h:17,
                 from include/linux/of_device.h:5,
                 from drivers/soc/fsl/qe/qe_common.c:19:
include/linux/ratelimit.h: In function 'ratelimit_state_init':
include/linux/ratelimit.h:14:21: error: passing argument 1 of '__raw_spin_lock_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
   14 |  raw_spin_lock_init(&rs->lock);
include/linux/spinlock.h:102:24: note: in definition of macro 'raw_spin_lock_init'
  102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
      |                        ^~~~
include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
   95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
      |                                    ~~~~~~~~~~~~~~~~^~~~

Caused by commit

  494c8512c90e ("printk: Make linux/printk.h self-contained")

changing include files is hadrer than it loooks :-(

I have used the printk tree from next-20200618 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Herbert Xu]

On Sun, Jun 21, 2020 at 01:15:54PM +1000, Stephen Rothwell wrote:
>
> Caused by commit
> 
>   494c8512c90e ("printk: Make linux/printk.h self-contained")
> 
> changing include files is hadrer than it loooks :-(

That's because this patch depends on the lockdep_types patch which
is in the x86 tree.  Petr, this patch needs to go on top of the
lockdep_types patch.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

Hi Stephen,

On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
> 
> After merging the printk tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from include/linux/printk.h:10,
>                  from include/linux/kernel.h:15,
>                  from include/linux/list.h:9,
>                  from include/linux/lockdep.h:43,
>                  from include/linux/spinlock_types.h:18,
>                  from include/linux/genalloc.h:32,
>                  from drivers/soc/fsl/qe/qe_common.c:16:
> include/linux/ratelimit_types.h:16:2: error: unknown type name 'raw_spinlock_t'
>    16 |  raw_spinlock_t lock;  /* protect the state */
>       |  ^~~~~~~~~~~~~~
> In file included from include/linux/wait.h:9,
>                  from include/linux/pid.h:6,
>                  from include/linux/sched.h:14,
>                  from include/linux/ratelimit.h:6,
>                  from include/linux/dev_printk.h:16,
>                  from include/linux/device.h:15,
>                  from include/linux/node.h:18,
>                  from include/linux/cpu.h:17,
>                  from include/linux/of_device.h:5,
>                  from drivers/soc/fsl/qe/qe_common.c:19:
> include/linux/ratelimit.h: In function 'ratelimit_state_init':
> include/linux/ratelimit.h:14:21: error: passing argument 1 of '__raw_spin_lock_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
>    14 |  raw_spin_lock_init(&rs->lock);
> include/linux/spinlock.h:102:24: note: in definition of macro 'raw_spin_lock_init'
>   102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
>       |                        ^~~~
> include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
>    95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
>       |                                    ~~~~~~~~~~~~~~~~^~~~
> 
> Caused by commit
> 
>   494c8512c90e ("printk: Make linux/printk.h self-contained")
> 
> changing include files is hadrer than it loooks :-(
> 
> I have used the printk tree from next-20200618 for today.

I am still getting this failure.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Petr Mladek]

On Tue 2020-06-23 10:26:55, Stephen Rothwell wrote:
> Hi Stephen,
> 
> On Sun, 21 Jun 2020 13:15:54 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> > 
> > After merging the printk tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > In file included from include/linux/printk.h:10,
> >                  from include/linux/kernel.h:15,
> >                  from include/linux/list.h:9,
> >                  from include/linux/lockdep.h:43,
> >                  from include/linux/spinlock_types.h:18,
> >                  from include/linux/genalloc.h:32,
> >                  from drivers/soc/fsl/qe/qe_common.c:16:
> > include/linux/ratelimit_types.h:16:2: error: unknown type name 'raw_spinlock_t'
> >    16 |  raw_spinlock_t lock;  /* protect the state */
> >       |  ^~~~~~~~~~~~~~
> > In file included from include/linux/wait.h:9,
> >                  from include/linux/pid.h:6,
> >                  from include/linux/sched.h:14,
> >                  from include/linux/ratelimit.h:6,
> >                  from include/linux/dev_printk.h:16,
> >                  from include/linux/device.h:15,
> >                  from include/linux/node.h:18,
> >                  from include/linux/cpu.h:17,
> >                  from include/linux/of_device.h:5,
> >                  from drivers/soc/fsl/qe/qe_common.c:19:
> > include/linux/ratelimit.h: In function 'ratelimit_state_init':
> > include/linux/ratelimit.h:14:21: error: passing argument 1 of '__raw_spin_lock_init' from incompatible pointer type [-Werror=incompatible-pointer-types]
> >    14 |  raw_spin_lock_init(&rs->lock);
> > include/linux/spinlock.h:102:24: note: in definition of macro 'raw_spin_lock_init'
> >   102 |  __raw_spin_lock_init((lock), #lock, &__key, LD_WAIT_SPIN); \
> >       |                        ^~~~
> > include/linux/spinlock.h:95:52: note: expected 'raw_spinlock_t *' {aka 'struct raw_spinlock *'} but argument is of type 'int *'
> >    95 |   extern void __raw_spin_lock_init(raw_spinlock_t *lock, const char *name,
> >       |                                    ~~~~~~~~~~~~~~~~^~~~
> > 
> > Caused by commit
> > 
> >   494c8512c90e ("printk: Make linux/printk.h self-contained")
> > 
> > changing include files is hadrer than it loooks :-(
> > 
> > I have used the printk tree from next-20200618 for today.
> 
> I am still getting this failure.

I have removed the problematic commit for now. It tried to remove
some cyclic dependencies from heavily used include files. It clearly
needs more love.

I am sorry for the late reaction. I have semi-lost mails from last 4 days.
I am still trying to recover them.

Best Regards,
Petr

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

[Herbert Xu]

On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote:
>
> I have removed the problematic commit for now. It tried to remove
> some cyclic dependencies from heavily used include files. It clearly
> needs more love.

Hmm, the cyclic dependencies are there because you didn't pull in
the lockdep_types patch.  The printk patch must go on top of the
lockdep_types patch.  How about just putting this into the x86 tree
alongside the lockdep_types patch?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Petr Mladek]

On Tue 2020-06-23 22:19:37, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 02:16:38PM +0200, Petr Mladek wrote:
> >
> > I have removed the problematic commit for now. It tried to remove
> > some cyclic dependencies from heavily used include files. It clearly
> > needs more love.
> 
> Hmm, the cyclic dependencies are there because you didn't pull in
> the lockdep_types patch.  The printk patch must go on top of the
> lockdep_types patch.  How about just putting this into the x86 tree
> alongside the lockdep_types patch?

I see the problem with both patches.

> In file included from include/linux/printk.h:10,
>                  from include/linux/kernel.h:15,
>                  from include/linux/list.h:9,
>                  from include/linux/lockdep.h:43,
>                  from include/linux/spinlock_types.h:18,
>                  from include/linux/genalloc.h:32,
>                  from drivers/soc/fsl/qe/qe_common.c:16:
> include/linux/ratelimit_types.h:16:2: error: unknown type name 'raw_spinlock_t'
>    16 |  raw_spinlock_t lock;  /* protect the state */

It is similar cycle:

spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h

But this time it happens via list.h -> kernel.h ->printk.h.
Where list.h needs READ_ONCE() stuff from compiler.h.


My "allmodconfig" build has successfully finished with the following extra
 fix on top of the two patches:

diff --git a/include/linux/list.h b/include/linux/list.h
index aff44d34f4e4..6d606c4036ce 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -6,7 +6,7 @@
 #include <linux/stddef.h>
 #include <linux/poison.h>
 #include <linux/const.h>
-#include <linux/kernel.h>
+#include <linux/compiler.h>
 
 /*
  * Simple doubly linked list implementation.

I suggest to bundle this into the 2nd patch that makes linux/printk.h
self-contained.

Best Regards,
Petr

PS: And yes, it makes sense to push both patches via a single tree to
make sure that the lockdep.h split is done first.

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

[Peter Zijlstra]

On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:

> PS: And yes, it makes sense to push both patches via a single tree to
> make sure that the lockdep.h split is done first.

That's what I got you tip/locking/header for, pull that topic branch
into your tree.

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

[Herbert Xu]

On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
>
> It is similar cycle:
> 
> spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> 
> But this time it happens via list.h -> kernel.h ->printk.h.
> Where list.h needs READ_ONCE() stuff from compiler.h.

But this is exactly the loop that's fixed by the lockdep_types
patch.  Did you get a compile failure with *just* the lockdep
patch?

> PS: And yes, it makes sense to push both patches via a single tree to
> make sure that the lockdep.h split is done first.

OK, can I repost this patch with your ack then?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Petr Mladek]

On Wed 2020-06-24 10:20:58, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> >
> > It is similar cycle:
> > 
> > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> > 
> > But this time it happens via list.h -> kernel.h ->printk.h.
> > Where list.h needs READ_ONCE() stuff from compiler.h.
> 
> But this is exactly the loop that's fixed by the lockdep_types
> patch.

I think that this problem is similar to the one on s390 that was
solved by compiler.h in arch/s390/include/asm/bug.h

It is related to
https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local

It seems that we need the same change also in list.h.


> Did you get a compile failure with *just* the lockdep
> patch?

No, I see the problem only with both patches.


> > PS: And yes, it makes sense to push both patches via a single tree to
> > make sure that the lockdep.h split is done first.
> 
> OK, can I repost this patch with your ack then?

Do you mean to repost the 2nd patch with the change in
include/linux/list.h?

This would be great.

Best Regards,
Petr

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

[Herbert Xu]

On Wed, Jun 24, 2020 at 10:19:29AM +0200, Petr Mladek wrote:
> On Wed 2020-06-24 10:20:58, Herbert Xu wrote:
> > On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> > >
> > > It is similar cycle:
> > > 
> > > spinlock_types.h -> lockdep.h -> printk.h -> ratelimit.h -> spinlock_types.h
> > > 
> > > But this time it happens via list.h -> kernel.h ->printk.h.
> > > Where list.h needs READ_ONCE() stuff from compiler.h.
> > 
> > But this is exactly the loop that's fixed by the lockdep_types
> > patch.
> 
> I think that this problem is similar to the one on s390 that was
> solved by compiler.h in arch/s390/include/asm/bug.h
> 
> It is related to
> https://lore.kernel.org/r/20200613142901.65xieioomt6bbqa6@ltop.local
> 
> It seems that we need the same change also in list.h.

Oh I see.  I misunderstood what you said in the first email.
 
> Do you mean to repost the 2nd patch with the change in
> include/linux/list.h?

OK let me test this and repost.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Petr Mladek]

On Tue 2020-06-23 17:17:38, Peter Zijlstra wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> 
> > PS: And yes, it makes sense to push both patches via a single tree to
> > make sure that the lockdep.h split is done first.
> 
> That's what I got you tip/locking/header for, pull that topic branch
> into your tree.

I see, I misunderstood it.
I am going to merge the branch once I get the updated 2nd patch.

Best Regards,
Petr

[PATCH] lockdep: Move list.h inclusion into lockdep.h

[Herbert Xu]

On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
>
> My "allmodconfig" build has successfully finished with the following extra
>  fix on top of the two patches:
> 
> diff --git a/include/linux/list.h b/include/linux/list.h
> index aff44d34f4e4..6d606c4036ce 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -6,7 +6,7 @@
>  #include <linux/stddef.h>
>  #include <linux/poison.h>
>  #include <linux/const.h>
> -#include <linux/kernel.h>
> +#include <linux/compiler.h>

Unfortunately this doesn't work because list.h actually does need
kernel.h for container_of.

However, we can easily fix the loop another way by removing list.h
from lockdep.h as it doesn't actually use any list macros/functions
but only the list type which is now in linux/types.h.

We could either fold this into the lockdep_types patch, or fold it
into the printk patch, or just leave it as a standalone patch.
What do you guys think?

---8<---
Currently lockdep_types.h includes list.h without actually using any
of its macros or functions.  All it needs are the type definitions
which were moved into types.h long ago.  This potentially causes
inclusion loops because both are included by many core header
files.

This patch moves the list.h inclusion into lockdep.h.  Note that
we could probably remove it completely but that could potentially
result in compile failures should any end users not include list.h
directly and also be unlucky enough to not get list.h via some other
header file.

Reported-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 3b73cf84f77d..b1ad5c045353 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -21,6 +21,7 @@ extern int lock_stat;
 #ifdef CONFIG_LOCKDEP
 
 #include <linux/linkage.h>
+#include <linux/list.h>
 #include <linux/debug_locks.h>
 #include <linux/stacktrace.h>
 
diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
index 7b9350624577..bb35b449f533 100644
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -32,8 +32,6 @@ enum lockdep_wait_type {
 
 #ifdef CONFIG_LOCKDEP
 
-#include <linux/list.h>
-
 /*
  * We'd rather not expose kernel/lockdep_states.h this wide, but we do need
  * the total number of states... :-(
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] lockdep: Move list.h inclusion into lockdep.h

[Andy Shevchenko]

On Wed, Jun 24, 2020 at 3:43 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:

...

> > -#include <linux/kernel.h>
> > +#include <linux/compiler.h>
>
> Unfortunately this doesn't work because list.h actually does need
> kernel.h for container_of.
>
> However, we can easily fix the loop another way by removing list.h
> from lockdep.h as it doesn't actually use any list macros/functions
> but only the list type which is now in linux/types.h.
>
> We could either fold this into the lockdep_types patch, or fold it
> into the printk patch, or just leave it as a standalone patch.
> What do you guys think?

Does lockdep_types include types? Then we are fine and it is the way to go.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] lockdep: Move list.h inclusion into lockdep.h

[Herbert Xu]

On Wed, Jun 24, 2020 at 03:49:23PM +0300, Andy Shevchenko wrote:
>
> Does lockdep_types include types? Then we are fine and it is the way to go.

Yes it already does.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] lockdep: Move list.h inclusion into lockdep.h

[Petr Mladek]

On Wed 2020-06-24 22:42:12, Herbert Xu wrote:
> On Tue, Jun 23, 2020 at 04:28:58PM +0200, Petr Mladek wrote:
> >
> > My "allmodconfig" build has successfully finished with the following extra
> >  fix on top of the two patches:
> > 
> > diff --git a/include/linux/list.h b/include/linux/list.h
> > index aff44d34f4e4..6d606c4036ce 100644
> > --- a/include/linux/list.h
> > +++ b/include/linux/list.h
> > @@ -6,7 +6,7 @@
> >  #include <linux/stddef.h>
> >  #include <linux/poison.h>
> >  #include <linux/const.h>
> > -#include <linux/kernel.h>
> > +#include <linux/compiler.h>
> 
> Unfortunately this doesn't work because list.h actually does need
> kernel.h for container_of.

Ah, I see.

> However, we can easily fix the loop another way by removing list.h
> from lockdep.h as it doesn't actually use any list macros/functions
> but only the list type which is now in linux/types.h.
> 
> We could either fold this into the lockdep_types patch, or fold it
> into the printk patch, or just leave it as a standalone patch.
> What do you guys think?

It logically belongs to the lockdep_types area.

I think that separate patch is the best solution so that Peter does
not need to rebase tip/locking/header.

> 
> ---8<---
> Currently lockdep_types.h includes list.h without actually using any
> of its macros or functions.  All it needs are the type definitions
> which were moved into types.h long ago.  This potentially causes
> inclusion loops because both are included by many core header
> files.
> 
> This patch moves the list.h inclusion into lockdep.h.  Note that
> we could probably remove it completely but that could potentially
> result in compile failures should any end users not include list.h
> directly and also be unlucky enough to not get list.h via some other
> header file.
> 
> Reported-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

It works with allmodconfig here, so feel free to use:

Tested-by: Petr Mladek <pmladek@suse.com>

Of course, it does not have much value. There might still be another
configuration or architecture that does not work but I would leave
this for test bots.

Best Regards,
Petr

> 
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 3b73cf84f77d..b1ad5c045353 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -21,6 +21,7 @@ extern int lock_stat;
>  #ifdef CONFIG_LOCKDEP
>  
>  #include <linux/linkage.h>
> +#include <linux/list.h>
>  #include <linux/debug_locks.h>
>  #include <linux/stacktrace.h>
>  
> diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
> index 7b9350624577..bb35b449f533 100644
> --- a/include/linux/lockdep_types.h
> +++ b/include/linux/lockdep_types.h
> @@ -32,8 +32,6 @@ enum lockdep_wait_type {
>  
>  #ifdef CONFIG_LOCKDEP
>  
> -#include <linux/list.h>
> -
>  /*
>   * We'd rather not expose kernel/lockdep_states.h this wide, but we do need
>   * the total number of states... :-(
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] lockdep: Move list.h inclusion into lockdep.h

[Herbert Xu]

On Thu, Jun 25, 2020 at 12:11:19PM +0200, Petr Mladek wrote:
>
> It works with allmodconfig here, so feel free to use:
> 
> Tested-by: Petr Mladek <pmladek@suse.com>
> 
> Of course, it does not have much value. There might still be another
> configuration or architecture that does not work but I would leave
> this for test bots.

Thanks Petr!

Peter Z, could you please apply this patch on top of the existing
one in tip/locking/header?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt