header circular dependencies

header circular dependencies

[Andy Shevchenko]

`make headerdep` is full of printk.h circular dependencies, like

include/kvm/arm_vgic.h:18: warning: recursive header inclusion
In file included from linux/printk.h,
                from linux/dynamic_debug.h:188
                from linux/printk.h:555 <-- here

I'm wondering if it's a false positive?
In either case, can we teach the headerdep not to complain by fixing
the culprit?

-- 
With Best Regards,
Andy Shevchenko

Re: header circular dependencies

[Andy Shevchenko]

On Wed, Dec 22, 2021 at 9:08 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> `make headerdep` is full of printk.h circular dependencies, like
>
> include/kvm/arm_vgic.h:18: warning: recursive header inclusion

For the clearness, 18 here is with one of my (not related to printk,h)
patches, but the line in question is inclusion of jump_label.h. In the
original code it's 16.

> In file included from linux/printk.h,
>                 from linux/dynamic_debug.h:188
>                 from linux/printk.h:555 <-- here
>
> I'm wondering if it's a false positive?
> In either case, can we teach the headerdep not to complain by fixing
> the culprit?

-- 
With Best Regards,
Andy Shevchenko

Re: header circular dependencies

[Petr Mladek]

On Wed 2021-12-22 21:08:39, Andy Shevchenko wrote:
> `make headerdep` is full of printk.h circular dependencies, like
> 
> include/kvm/arm_vgic.h:18: warning: recursive header inclusion
> In file included from linux/printk.h,
>                 from linux/dynamic_debug.h:188
>                 from linux/printk.h:555 <-- here

This one looks like false positive:

   + printk.h includes dynamic_debug.h when CONFIG_DYNAMIC_DEBUG_CORE
   + dynamic_debug.h includes printk.h when !CONFIG_DYNAMIC_DEBUG_CORE

But there seem to be other cycles, for example:

   + printk.h
     + dynamic_debug.h
       + jump_label.h
	 + bug.h
	   + asm/bug.h
	     + printk.h

I guess that it somehow works _only_ because printk.h includes
dynamic_debug.h late. It probably defines everything that is needed
by bug.h early enough.

> I'm wondering if it's a false positive?
> In either case, can we teach the headerdep not to complain by fixing
> the culprit?

I am scratching my head how to clean this up. All the dependencies
make sense. The main problem is that all headers provide a lot of
inlined functionality. The inlining is often important either because
of speed or because every caller needs to have its own data structure
(struct _ddebug, struct static_key).

I can't find any good solution at the moment. But I am still slowed
down after the holidays.

Best Regards,
Petr

Re: header circular dependencies

[Andy Shevchenko]

On Tue, Jan 4, 2022 at 4:46 PM Petr Mladek <pmladek@suse.com> wrote:
>
> On Wed 2021-12-22 21:08:39, Andy Shevchenko wrote:
> > `make headerdep` is full of printk.h circular dependencies, like
> >
> > include/kvm/arm_vgic.h:18: warning: recursive header inclusion
> > In file included from linux/printk.h,
> >                 from linux/dynamic_debug.h:188
> >                 from linux/printk.h:555 <-- here
>
> This one looks like false positive:
>
>    + printk.h includes dynamic_debug.h when CONFIG_DYNAMIC_DEBUG_CORE
>    + dynamic_debug.h includes printk.h when !CONFIG_DYNAMIC_DEBUG_CORE
>
> But there seem to be other cycles, for example:
>
>    + printk.h
>      + dynamic_debug.h
>        + jump_label.h
>          + bug.h
>            + asm/bug.h
>              + printk.h
>
> I guess that it somehow works _only_ because printk.h includes
> dynamic_debug.h late. It probably defines everything that is needed
> by bug.h early enough.
>
> > I'm wondering if it's a false positive?
> > In either case, can we teach the headerdep not to complain by fixing
> > the culprit?
>
> I am scratching my head how to clean this up. All the dependencies
> make sense. The main problem is that all headers provide a lot of
> inlined functionality. The inlining is often important either because
> of speed or because every caller needs to have its own data structure
> (struct _ddebug, struct static_key).
>
> I can't find any good solution at the moment. But I am still slowed
> down after the holidays.

Thanks for looking into this.

Meanwhile (I haven't looked at that myself) Ingo announced some of his
long work on the topic:
https://lore.kernel.org/lkml/YdIfz+LMewetSaEB@gmail.com/

Perhaps he has a solution there.


-- 
With Best Regards,
Andy Shevchenko

Re: header circular dependencies

[Petr Mladek]

On Tue 2022-01-04 16:52:25, Andy Shevchenko wrote:
> On Tue, Jan 4, 2022 at 4:46 PM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Wed 2021-12-22 21:08:39, Andy Shevchenko wrote:
> > > `make headerdep` is full of printk.h circular dependencies, like
> > >
> > > include/kvm/arm_vgic.h:18: warning: recursive header inclusion
> > > In file included from linux/printk.h,
> > >                 from linux/dynamic_debug.h:188
> > >                 from linux/printk.h:555 <-- here
> >
> > This one looks like false positive:
> >
> >    + printk.h includes dynamic_debug.h when CONFIG_DYNAMIC_DEBUG_CORE
> >    + dynamic_debug.h includes printk.h when !CONFIG_DYNAMIC_DEBUG_CORE
> >
> > But there seem to be other cycles, for example:
> >
> >    + printk.h
> >      + dynamic_debug.h
> >        + jump_label.h
> >          + bug.h
> >            + asm/bug.h
> >              + printk.h
> >
> > I guess that it somehow works _only_ because printk.h includes
> > dynamic_debug.h late. It probably defines everything that is needed
> > by bug.h early enough.
> >
> > > I'm wondering if it's a false positive?
> > > In either case, can we teach the headerdep not to complain by fixing
> > > the culprit?
> >
> > I am scratching my head how to clean this up. All the dependencies
> > make sense. The main problem is that all headers provide a lot of
> > inlined functionality. The inlining is often important either because
> > of speed or because every caller needs to have its own data structure
> > (struct _ddebug, struct static_key).
> >
> > I can't find any good solution at the moment. But I am still slowed
> > down after the holidays.
> 
> Thanks for looking into this.
> 
> Meanwhile (I haven't looked at that myself) Ingo announced some of his
> long work on the topic:
> https://lore.kernel.org/lkml/YdIfz+LMewetSaEB@gmail.com/
> 
> Perhaps he has a solution there.

Thanks for the pointer. Unfortunately, it seems that Ingo's patchset
does not solve our problem. It is possible that printk.h did not look
interesting enough. printk() is used almost anywhere and probably
does not include that much stuff on its own.

Anyway, Ingo's patchset uses many tricks. It might be good inspiration
how to solve our dependency hell. I am going to think more about it.

Best Regards,
Petr