kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

[Johan Hovold]

On Wed, Dec 18, 2019 at 09:38:42AM +0100, Uwe Kleine-König wrote:
> Commit 54edb425346a ("serdev: simplify Makefile") broke builds with
> serdev configured as module. I don't understand it completely yet, but
> it seems that
> 
> 	obj-$(CONFIG_SERIAL_DEV_BUS) += serdev/
> 
> in drivers/tty/Makefile with CONFIG_SERIAL_DEV_BUS=m doesn't result in
> code that is added using obj-y in drivers/tty/serdev/Makefile being
> compiled. So instead of dropping $(CONFIG_SERIAL_DEV_BUS) in serdev's
> Makefile, drop it in drivers/tty/Makefile.

I was just looking into this when you commented on this off-list.

The offending patch is broken since it effectively makes
CONFIG_SERIAL_DEV_BUS bool (built-in or disabled), but for some reason
those symbols do not end up in vmlinux (despite being compiled) when you
add a built-in object goal under a directory that was entered using
obj-m.

That seems like a bug to me and contradicts the kbuild documentation
(3.6):

	Example:

	#fs/Makefile
	obj-$(CONFIG_EXT2_FS) += ext2/

	If CONFIG_EXT2_FS is set to either ‘y’ (built-in) or ‘m’
	(modular) the corresponding obj- variable will be set, and
	kbuild will descend down in the ext2 directory. Kbuild only uses
	this information to decide that it needs to visit the directory,
	it is the Makefile in the subdirectory that specifies what is
	modular and what is built-in.

I tried adding other targets to obj-y directly and they are also are not
included, seemingly since the directory was entered using obj-m.

Masahiro or Michal, can you shed some light?

Johan

Re: kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

[Masahiro Yamada]

Hi.

On Thu, Dec 19, 2019 at 2:23 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Dec 18, 2019 at 09:38:42AM +0100, Uwe Kleine-König wrote:
> > Commit 54edb425346a ("serdev: simplify Makefile") broke builds with
> > serdev configured as module. I don't understand it completely yet, but
> > it seems that
> >
> >       obj-$(CONFIG_SERIAL_DEV_BUS) += serdev/
> >
> > in drivers/tty/Makefile with CONFIG_SERIAL_DEV_BUS=m doesn't result in
> > code that is added using obj-y in drivers/tty/serdev/Makefile being
> > compiled. So instead of dropping $(CONFIG_SERIAL_DEV_BUS) in serdev's
> > Makefile, drop it in drivers/tty/Makefile.
>
> I was just looking into this when you commented on this off-list.
>
> The offending patch is broken since it effectively makes
> CONFIG_SERIAL_DEV_BUS bool (built-in or disabled), but for some reason
> those symbols do not end up in vmlinux (despite being compiled) when you
> add a built-in object goal under a directory that was entered using
> obj-m.
>
> That seems like a bug to me and contradicts the kbuild documentation
> (3.6):
>
>         Example:
>
>         #fs/Makefile
>         obj-$(CONFIG_EXT2_FS) += ext2/
>
>         If CONFIG_EXT2_FS is set to either ‘y’ (built-in) or ‘m’
>         (modular) the corresponding obj- variable will be set, and
>         kbuild will descend down in the ext2 directory. Kbuild only uses
>         this information to decide that it needs to visit the directory,
>         it is the Makefile in the subdirectory that specifies what is
>         modular and what is built-in.
>
> I tried adding other targets to obj-y directly and they are also are not
> included, seemingly since the directory was entered using obj-m.
>
> Masahiro or Michal, can you shed some light?
>
> Johan


I saw similar questions in ML in the past.
Yes, this is how Kbuild works;
Kbuild only links objects in Makefiles visited by obj-y.

If you use
    obj-m += serdev/
all objects in serdev/ are considered as modular.

I think it is better to make the document
clarify this.


-- 
Best Regards
Masahiro Yamada

Re: kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

[Johan Hovold]

On Thu, Dec 19, 2019 at 11:00:15AM +0900, Masahiro Yamada wrote:
> Hi.
> 
> On Thu, Dec 19, 2019 at 2:23 AM Johan Hovold <johan@kernel.org> wrote:

> > The offending patch is broken since it effectively makes
> > CONFIG_SERIAL_DEV_BUS bool (built-in or disabled), but for some reason
> > those symbols do not end up in vmlinux (despite being compiled) when you
> > add a built-in object goal under a directory that was entered using
> > obj-m.
> >
> > That seems like a bug to me and contradicts the kbuild documentation
> > (3.6):
> >
> >         Example:
> >
> >         #fs/Makefile
> >         obj-$(CONFIG_EXT2_FS) += ext2/
> >
> >         If CONFIG_EXT2_FS is set to either ‘y’ (built-in) or ‘m’
> >         (modular) the corresponding obj- variable will be set, and
> >         kbuild will descend down in the ext2 directory. Kbuild only uses
> >         this information to decide that it needs to visit the directory,
> >         it is the Makefile in the subdirectory that specifies what is
> >         modular and what is built-in.
> >
> > I tried adding other targets to obj-y directly and they are also are not
> > included, seemingly since the directory was entered using obj-m.
> >
> > Masahiro or Michal, can you shed some light?

> I saw similar questions in ML in the past.
> Yes, this is how Kbuild works;
> Kbuild only links objects in Makefiles visited by obj-y.
> 
> If you use
>     obj-m += serdev/
> all objects in serdev/ are considered as modular.

Well, any objects in serdev/ specified in obj-y would currently be built
but never included in either built-in.a or any module.

> I think it is better to make the document
> clarify this.

Yeah, I don't have a use case for this, but the documentation would need
to be updated to not have more people look into this.

And at least this behaviour allowed us to catch this bug, but only
because we had dependent modules that failed to build. I see now that
you posted a patch adding a general warning about obj-y under obj-m last
fall, but that was apparently never merged.

But when was this behaviour changed? And was that done on purpose?

Johan

Re: kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

[Masahiro Yamada]

On Thu, Dec 19, 2019 at 5:18 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Dec 19, 2019 at 11:00:15AM +0900, Masahiro Yamada wrote:
> > Hi.
> >
> > On Thu, Dec 19, 2019 at 2:23 AM Johan Hovold <johan@kernel.org> wrote:
>
> > > The offending patch is broken since it effectively makes
> > > CONFIG_SERIAL_DEV_BUS bool (built-in or disabled), but for some reason
> > > those symbols do not end up in vmlinux (despite being compiled) when you
> > > add a built-in object goal under a directory that was entered using
> > > obj-m.
> > >
> > > That seems like a bug to me and contradicts the kbuild documentation
> > > (3.6):
> > >
> > >         Example:
> > >
> > >         #fs/Makefile
> > >         obj-$(CONFIG_EXT2_FS) += ext2/
> > >
> > >         If CONFIG_EXT2_FS is set to either ‘y’ (built-in) or ‘m’
> > >         (modular) the corresponding obj- variable will be set, and
> > >         kbuild will descend down in the ext2 directory. Kbuild only uses
> > >         this information to decide that it needs to visit the directory,
> > >         it is the Makefile in the subdirectory that specifies what is
> > >         modular and what is built-in.
> > >
> > > I tried adding other targets to obj-y directly and they are also are not
> > > included, seemingly since the directory was entered using obj-m.
> > >
> > > Masahiro or Michal, can you shed some light?
>
> > I saw similar questions in ML in the past.
> > Yes, this is how Kbuild works;
> > Kbuild only links objects in Makefiles visited by obj-y.
> >
> > If you use
> >     obj-m += serdev/
> > all objects in serdev/ are considered as modular.
>
> Well, any objects in serdev/ specified in obj-y would currently be built
> but never included in either built-in.a or any module.

Right.

>
> > I think it is better to make the document
> > clarify this.
>
> Yeah, I don't have a use case for this, but the documentation would need
> to be updated to not have more people look into this.
>
> And at least this behaviour allowed us to catch this bug, but only
> because we had dependent modules that failed to build. I see now that
> you posted a patch adding a general warning about obj-y under obj-m last
> fall, but that was apparently never merged.

This one?

https://lore.kernel.org/patchwork/patch/1126959/

This patch detected several Makefile/Kconfig bugs,
and it is good.

But, it also turned out to cause false positive warnings
on some architectures.
I thought it was not so easy to fix it.
So, I decided to postpone it.


> But when was this behaviour changed? And was that done on purpose?

No. Not changed at all.

In my understanding, the current behavior has been kept
from the beginning.



> Johan



-- 
Best Regards
Masahiro Yamada

Re: kbuild obj-m directory descend (was: Re: [PATCH] serdev: fix builds with CONFIG_SERIAL_DEV_BUS=m)

[Johan Hovold]

On Thu, Dec 19, 2019 at 05:55:54PM +0900, Masahiro Yamada wrote:
> On Thu, Dec 19, 2019 at 5:18 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Thu, Dec 19, 2019 at 11:00:15AM +0900, Masahiro Yamada wrote:
> > > Hi.
> > >
> > > On Thu, Dec 19, 2019 at 2:23 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > > > The offending patch is broken since it effectively makes
> > > > CONFIG_SERIAL_DEV_BUS bool (built-in or disabled), but for some reason
> > > > those symbols do not end up in vmlinux (despite being compiled) when you
> > > > add a built-in object goal under a directory that was entered using
> > > > obj-m.
> > > >
> > > > That seems like a bug to me and contradicts the kbuild documentation
> > > > (3.6):
> > > >
> > > >         Example:
> > > >
> > > >         #fs/Makefile
> > > >         obj-$(CONFIG_EXT2_FS) += ext2/
> > > >
> > > >         If CONFIG_EXT2_FS is set to either ‘y’ (built-in) or ‘m’
> > > >         (modular) the corresponding obj- variable will be set, and
> > > >         kbuild will descend down in the ext2 directory. Kbuild only uses
> > > >         this information to decide that it needs to visit the directory,
> > > >         it is the Makefile in the subdirectory that specifies what is
> > > >         modular and what is built-in.
> > > >
> > > > I tried adding other targets to obj-y directly and they are also are not
> > > > included, seemingly since the directory was entered using obj-m.
> > > >
> > > > Masahiro or Michal, can you shed some light?
> >
> > > I saw similar questions in ML in the past.
> > > Yes, this is how Kbuild works;
> > > Kbuild only links objects in Makefiles visited by obj-y.
> > >
> > > If you use
> > >     obj-m += serdev/
> > > all objects in serdev/ are considered as modular.
> >
> > Well, any objects in serdev/ specified in obj-y would currently be built
> > but never included in either built-in.a or any module.
> 
> Right.
> 
> > > I think it is better to make the document
> > > clarify this.
> >
> > Yeah, I don't have a use case for this, but the documentation would need
> > to be updated to not have more people look into this.
> >
> > And at least this behaviour allowed us to catch this bug, but only
> > because we had dependent modules that failed to build. I see now that
> > you posted a patch adding a general warning about obj-y under obj-m last
> > fall, but that was apparently never merged.
> 
> This one?
> 
> https://lore.kernel.org/patchwork/patch/1126959/

Yep, that's the one I meant.
 
> This patch detected several Makefile/Kconfig bugs,
> and it is good.
> 
> But, it also turned out to cause false positive warnings
> on some architectures.
> I thought it was not so easy to fix it.
> So, I decided to postpone it.

Ok.

> > But when was this behaviour changed? And was that done on purpose?
> 
> No. Not changed at all.
> 
> In my understanding, the current behavior has been kept
> from the beginning.

Ok, thanks for confirming. The contradicting documentation I refer to
above has been there since before git at least however.

Here's a random example of someone being bit by this five years ago:

	https://www.spinics.net/lists/linux-kbuild/msg10887.html

Would you mind taking a stab at updating the docs?

Johan