All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kbuild: add extra-y to targets-for-modules
@ 2020-11-03  5:44 WANG Chao
  2020-11-23  5:23 ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: WANG Chao @ 2020-11-03  5:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masahiro Yamada, linux-kbuild

extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
("kbuild: do not create built-in objects for external module builds").

This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
and 'make M=...' to build livepatch patch module.

Add extra-y to targets-for-modules so that such kind of build works
properly.

Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
---
 scripts/Makefile.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ae647379b579..0113a042d643 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -86,7 +86,7 @@ ifdef need-builtin
 targets-for-builtin += $(obj)/built-in.a
 endif
 
-targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
 
 ifdef need-modorder
 targets-for-modules += $(obj)/modules.order
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-11-03  5:44 [PATCH] kbuild: add extra-y to targets-for-modules WANG Chao
@ 2020-11-23  5:23 ` Masahiro Yamada
  2020-11-23 15:04   ` WANG Chao
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2020-11-23  5:23 UTC (permalink / raw)
  To: WANG Chao; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list

On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
>
> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> ("kbuild: do not create built-in objects for external module builds").
>
> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> and 'make M=...' to build livepatch patch module.
>
> Add extra-y to targets-for-modules so that such kind of build works
> properly.
>
> Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> ---
>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index ae647379b579..0113a042d643 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -86,7 +86,7 @@ ifdef need-builtin
>  targets-for-builtin += $(obj)/built-in.a
>  endif
>
> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>
>  ifdef need-modorder
>  targets-for-modules += $(obj)/modules.order
> --
> 2.29.1
>

NACK.

Please fix your Makefile.

Hint:
https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/


Probably what you should use is 'targets'.







-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-11-23  5:23 ` Masahiro Yamada
@ 2020-11-23 15:04   ` WANG Chao
  2020-11-25 13:42     ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: WANG Chao @ 2020-11-23 15:04 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list

On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> >
> > extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > ("kbuild: do not create built-in objects for external module builds").
> >
> > This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > and 'make M=...' to build livepatch patch module.
> >
> > Add extra-y to targets-for-modules so that such kind of build works
> > properly.
> >
> > Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > ---
> >  scripts/Makefile.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index ae647379b579..0113a042d643 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -86,7 +86,7 @@ ifdef need-builtin
> >  targets-for-builtin += $(obj)/built-in.a
> >  endif
> >
> > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> >
> >  ifdef need-modorder
> >  targets-for-modules += $(obj)/modules.order
> > --
> > 2.29.1
> >
> 
> NACK.
> 
> Please fix your Makefile.
> 
> Hint:
> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> 
> 
> Probably what you should use is 'targets'.

I tried with 'targets' and 'always-y'. Both doesn't work for me.

I narraw it down to the following example:

cat > Makefile << _EOF_
obj-m += foo.o

ldflags-y += -T $(src)/kpatch.lds
always-y += kpatch.lds

foo-objs += bar.o

all:
	make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
clean:
	make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
_EOF_

Take a look into scripts/Makefile.build:488:

__build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
	 $(if $(KBUILD_MODULES), $(targets-for-modules)) \
	 $(subdir-ym) $(always-y)
	@:

'always-y' is built after 'targets-for-modules'. This makes
'targets-for-modules' fails because kpatch.lds isn't there.

For 'targets', in case of OOT, does not seem to be useful.

What change do you suggest to make to fix this kind of Makefile?

Thanks,
WANG Chao

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-11-23 15:04   ` WANG Chao
@ 2020-11-25 13:42     ` Masahiro Yamada
  2020-12-08  9:20       ` WANG Chao
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2020-11-25 13:42 UTC (permalink / raw)
  To: WANG Chao; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list

On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
>
> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >
> > > extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > > ("kbuild: do not create built-in objects for external module builds").
> > >
> > > This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > > and 'make M=...' to build livepatch patch module.
> > >
> > > Add extra-y to targets-for-modules so that such kind of build works
> > > properly.
> > >
> > > Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > > ---
> > >  scripts/Makefile.build | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index ae647379b579..0113a042d643 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -86,7 +86,7 @@ ifdef need-builtin
> > >  targets-for-builtin += $(obj)/built-in.a
> > >  endif
> > >
> > > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > >
> > >  ifdef need-modorder
> > >  targets-for-modules += $(obj)/modules.order
> > > --
> > > 2.29.1
> > >
> >
> > NACK.
> >
> > Please fix your Makefile.
> >
> > Hint:
> > https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> >
> >
> > Probably what you should use is 'targets'.
>
> I tried with 'targets' and 'always-y'. Both doesn't work for me.
>
> I narraw it down to the following example:
>
> cat > Makefile << _EOF_
> obj-m += foo.o
>
> ldflags-y += -T $(src)/kpatch.lds
> always-y += kpatch.lds
>
> foo-objs += bar.o
>
> all:
>         make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> clean:
>         make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> _EOF_
>
> Take a look into scripts/Makefile.build:488:
>
> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
>          $(if $(KBUILD_MODULES), $(targets-for-modules)) \
>          $(subdir-ym) $(always-y)
>         @:
>
> 'always-y' is built after 'targets-for-modules'. This makes
> 'targets-for-modules' fails because kpatch.lds isn't there.


Heh, you rely on the targets built from left to right,
and you have never thought Make supports the parallel option -j.


You need to specify the dependency if you expect objects
are built in the particular order.

However, in this case, using ldflags-y looks wrong
in the first place.

The linker script is used when combining the object
as well as the final link of *.ko


> For 'targets', in case of OOT, does not seem to be useful.
>
> What change do you suggest to make to fix this kind of Makefile?
>
> Thanks,
> WANG Chao



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-11-25 13:42     ` Masahiro Yamada
@ 2020-12-08  9:20       ` WANG Chao
  2020-12-08 14:31         ` Artem Savkov
  0 siblings, 1 reply; 11+ messages in thread
From: WANG Chao @ 2020-12-08  9:20 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kernel Mailing List, Linux Kbuild mailing list,
	artem.savkov, joe.lawrence

Sorry for the late reply.

On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> >
> > On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > > On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > > >
> > > > extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > > > ("kbuild: do not create built-in objects for external module builds").
> > > >
> > > > This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > > > and 'make M=...' to build livepatch patch module.
> > > >
> > > > Add extra-y to targets-for-modules so that such kind of build works
> > > > properly.
> > > >
> > > > Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > > > ---
> > > >  scripts/Makefile.build | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > > index ae647379b579..0113a042d643 100644
> > > > --- a/scripts/Makefile.build
> > > > +++ b/scripts/Makefile.build
> > > > @@ -86,7 +86,7 @@ ifdef need-builtin
> > > >  targets-for-builtin += $(obj)/built-in.a
> > > >  endif
> > > >
> > > > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > > +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > >
> > > >  ifdef need-modorder
> > > >  targets-for-modules += $(obj)/modules.order
> > > > --
> > > > 2.29.1
> > > >
> > >
> > > NACK.
> > >
> > > Please fix your Makefile.
> > >
> > > Hint:
> > > https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> > >
> > >
> > > Probably what you should use is 'targets'.
> >
> > I tried with 'targets' and 'always-y'. Both doesn't work for me.
> >
> > I narraw it down to the following example:
> >
> > cat > Makefile << _EOF_
> > obj-m += foo.o
> >
> > ldflags-y += -T $(src)/kpatch.lds
> > always-y += kpatch.lds
> >
> > foo-objs += bar.o
> >
> > all:
> >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> > clean:
> >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> > _EOF_
> >
> > Take a look into scripts/Makefile.build:488:
> >
> > __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> >          $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> >          $(subdir-ym) $(always-y)
> >         @:
> >
> > 'always-y' is built after 'targets-for-modules'. This makes
> > 'targets-for-modules' fails because kpatch.lds isn't there.
> 
> 
> Heh, you rely on the targets built from left to right,
> and you have never thought Make supports the parallel option -j.

You're right. I missed that.

> 
> 
> You need to specify the dependency if you expect objects
> are built in the particular order.
> 
> However, in this case, using ldflags-y looks wrong
> in the first place.
> 
> The linker script is used when combining the object
> as well as the final link of *.ko

I don't have a clean fix to kpatch right now.

I'm looping kpatch forks in. They're also looking at this right now:

https://github.com/dynup/kpatch/pull/1149

Thanks
WANG Chao

> 
> 
> > For 'targets', in case of OOT, does not seem to be useful.
> >
> > What change do you suggest to make to fix this kind of Makefile?
> >
> > Thanks,
> > WANG Chao
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-08  9:20       ` WANG Chao
@ 2020-12-08 14:31         ` Artem Savkov
  2020-12-16  6:14           ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: Artem Savkov @ 2020-12-08 14:31 UTC (permalink / raw)
  To: WANG Chao
  Cc: Masahiro Yamada, Linux Kernel Mailing List,
	Linux Kbuild mailing list, joe.lawrence

On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
> Sorry for the late reply.
> 
> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> > On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >
> > > On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > > > On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > > > >
> > > > > extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > > > > ("kbuild: do not create built-in objects for external module builds").
> > > > >
> > > > > This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > > > > and 'make M=...' to build livepatch patch module.
> > > > >
> > > > > Add extra-y to targets-for-modules so that such kind of build works
> > > > > properly.
> > > > >
> > > > > Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > > > > ---
> > > > >  scripts/Makefile.build | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > > > index ae647379b579..0113a042d643 100644
> > > > > --- a/scripts/Makefile.build
> > > > > +++ b/scripts/Makefile.build
> > > > > @@ -86,7 +86,7 @@ ifdef need-builtin
> > > > >  targets-for-builtin += $(obj)/built-in.a
> > > > >  endif
> > > > >
> > > > > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > > > +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > > >
> > > > >  ifdef need-modorder
> > > > >  targets-for-modules += $(obj)/modules.order
> > > > > --
> > > > > 2.29.1
> > > > >
> > > >
> > > > NACK.
> > > >
> > > > Please fix your Makefile.
> > > >
> > > > Hint:
> > > > https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> > > >
> > > >
> > > > Probably what you should use is 'targets'.
> > >
> > > I tried with 'targets' and 'always-y'. Both doesn't work for me.
> > >
> > > I narraw it down to the following example:
> > >
> > > cat > Makefile << _EOF_
> > > obj-m += foo.o
> > >
> > > ldflags-y += -T $(src)/kpatch.lds
> > > always-y += kpatch.lds
> > >
> > > foo-objs += bar.o
> > >
> > > all:
> > >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> > > clean:
> > >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> > > _EOF_
> > >
> > > Take a look into scripts/Makefile.build:488:
> > >
> > > __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> > >          $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> > >          $(subdir-ym) $(always-y)
> > >         @:
> > >
> > > 'always-y' is built after 'targets-for-modules'. This makes
> > > 'targets-for-modules' fails because kpatch.lds isn't there.
> > 
> > 
> > Heh, you rely on the targets built from left to right,
> > and you have never thought Make supports the parallel option -j.
> 
> You're right. I missed that.
> 
> > 
> > 
> > You need to specify the dependency if you expect objects
> > are built in the particular order.
> > 
> > However, in this case, using ldflags-y looks wrong
> > in the first place.
> > 
> > The linker script is used when combining the object
> > as well as the final link of *.ko

We want linker script to be used on both those steps, otherwise modpost
fails.

It looks like the right thing to do here is leave ldflags-y in, get rid
of always-y/extra-y altogether and specify our linker script as a
dependency for the object.

> I don't have a clean fix to kpatch right now.
> 
> I'm looping kpatch forks in. They're also looking at this right now:
> 
> https://github.com/dynup/kpatch/pull/1149
> 
> Thanks
> WANG Chao
> 
> > 
> > 
> > > For 'targets', in case of OOT, does not seem to be useful.
> > >
> > > What change do you suggest to make to fix this kind of Makefile?
> > >
> > > Thanks,
> > > WANG Chao
> > 
> > 
> > 
> > -- 
> > Best Regards
> > Masahiro Yamada
> > 
> 

-- 
Regards,
  Artem Savkov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-08 14:31         ` Artem Savkov
@ 2020-12-16  6:14           ` Masahiro Yamada
  2020-12-16 23:04             ` Joe Lawrence
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2020-12-16  6:14 UTC (permalink / raw)
  To: Artem Savkov
  Cc: WANG Chao, Linux Kernel Mailing List, Linux Kbuild mailing list,
	Joe Lawrence

On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@gmail.com> wrote:
>
> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
> > Sorry for the late reply.
> >
> > On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> > > On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> > > >
> > > > On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > > > > On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > > > > >
> > > > > > extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > > > > > ("kbuild: do not create built-in objects for external module builds").
> > > > > >
> > > > > > This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > > > > > and 'make M=...' to build livepatch patch module.
> > > > > >
> > > > > > Add extra-y to targets-for-modules so that such kind of build works
> > > > > > properly.
> > > > > >
> > > > > > Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > > > > > ---
> > > > > >  scripts/Makefile.build | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > > > > index ae647379b579..0113a042d643 100644
> > > > > > --- a/scripts/Makefile.build
> > > > > > +++ b/scripts/Makefile.build
> > > > > > @@ -86,7 +86,7 @@ ifdef need-builtin
> > > > > >  targets-for-builtin += $(obj)/built-in.a
> > > > > >  endif
> > > > > >
> > > > > > -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > > > > +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > > > > >
> > > > > >  ifdef need-modorder
> > > > > >  targets-for-modules += $(obj)/modules.order
> > > > > > --
> > > > > > 2.29.1
> > > > > >
> > > > >
> > > > > NACK.
> > > > >
> > > > > Please fix your Makefile.
> > > > >
> > > > > Hint:
> > > > > https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> > > > >
> > > > >
> > > > > Probably what you should use is 'targets'.
> > > >
> > > > I tried with 'targets' and 'always-y'. Both doesn't work for me.
> > > >
> > > > I narraw it down to the following example:
> > > >
> > > > cat > Makefile << _EOF_
> > > > obj-m += foo.o
> > > >
> > > > ldflags-y += -T $(src)/kpatch.lds
> > > > always-y += kpatch.lds
> > > >
> > > > foo-objs += bar.o
> > > >
> > > > all:
> > > >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> > > > clean:
> > > >         make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> > > > _EOF_
> > > >
> > > > Take a look into scripts/Makefile.build:488:
> > > >
> > > > __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> > > >          $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> > > >          $(subdir-ym) $(always-y)
> > > >         @:
> > > >
> > > > 'always-y' is built after 'targets-for-modules'. This makes
> > > > 'targets-for-modules' fails because kpatch.lds isn't there.
> > >
> > >
> > > Heh, you rely on the targets built from left to right,
> > > and you have never thought Make supports the parallel option -j.
> >
> > You're right. I missed that.
> >
> > >
> > >
> > > You need to specify the dependency if you expect objects
> > > are built in the particular order.
> > >
> > > However, in this case, using ldflags-y looks wrong
> > > in the first place.
> > >
> > > The linker script is used when combining the object
> > > as well as the final link of *.ko
>
> We want linker script to be used on both those steps, otherwise modpost
> fails.


In that case, does the following work?
(untested)



diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
index e017b17..02d4c66 100644
--- a/kmod/patch/Makefile
+++ b/kmod/patch/Makefile
@@ -12,7 +12,9 @@ endif

 obj-m += $(KPATCH_NAME).o
 ldflags-y += -T $(src)/kpatch.lds
-extra-y := kpatch.lds
+targets += kpatch.lds
+
+$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds

 $(KPATCH_NAME)-objs += patch-hook.o output.o





>
> It looks like the right thing to do here is leave ldflags-y in, get rid
> of always-y/extra-y altogether and specify our linker script as a
> dependency for the object.
>
> > I don't have a clean fix to kpatch right now.
> >
> > I'm looping kpatch forks in. They're also looking at this right now:
> >
> > https://github.com/dynup/kpatch/pull/1149
> >
> > Thanks
> > WANG Chao
> >
> > >
> > >
> > > > For 'targets', in case of OOT, does not seem to be useful.
> > > >
> > > > What change do you suggest to make to fix this kind of Makefile?
> > > >
> > > > Thanks,
> > > > WANG Chao
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Masahiro Yamada
> > >
> >
>
> --
> Regards,
>   Artem Savkov



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-16  6:14           ` Masahiro Yamada
@ 2020-12-16 23:04             ` Joe Lawrence
  2020-12-17  8:26               ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Lawrence @ 2020-12-16 23:04 UTC (permalink / raw)
  To: Masahiro Yamada, Artem Savkov
  Cc: WANG Chao, Linux Kernel Mailing List, Linux Kbuild mailing list

On 12/16/20 1:14 AM, Masahiro Yamada wrote:
> On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@gmail.com> wrote:
>>
>> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
>>> Sorry for the late reply.
>>>
>>> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
>>>> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
>>>>>
>>>>> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
>>>>>> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
>>>>>>>
>>>>>>> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
>>>>>>> ("kbuild: do not create built-in objects for external module builds").
>>>>>>>
>>>>>>> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
>>>>>>> and 'make M=...' to build livepatch patch module.
>>>>>>>
>>>>>>> Add extra-y to targets-for-modules so that such kind of build works
>>>>>>> properly.
>>>>>>>
>>>>>>> Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
>>>>>>> ---
>>>>>>>   scripts/Makefile.build | 2 +-
>>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>>>>>>> index ae647379b579..0113a042d643 100644
>>>>>>> --- a/scripts/Makefile.build
>>>>>>> +++ b/scripts/Makefile.build
>>>>>>> @@ -86,7 +86,7 @@ ifdef need-builtin
>>>>>>>   targets-for-builtin += $(obj)/built-in.a
>>>>>>>   endif
>>>>>>>
>>>>>>> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>>>>>>> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>>>>>>>
>>>>>>>   ifdef need-modorder
>>>>>>>   targets-for-modules += $(obj)/modules.order
>>>>>>> --
>>>>>>> 2.29.1
>>>>>>>
>>>>>>
>>>>>> NACK.
>>>>>>
>>>>>> Please fix your Makefile.
>>>>>>
>>>>>> Hint:
>>>>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
>>>>>>
>>>>>>
>>>>>> Probably what you should use is 'targets'.
>>>>>
>>>>> I tried with 'targets' and 'always-y'. Both doesn't work for me.
>>>>>
>>>>> I narraw it down to the following example:
>>>>>
>>>>> cat > Makefile << _EOF_
>>>>> obj-m += foo.o
>>>>>
>>>>> ldflags-y += -T $(src)/kpatch.lds
>>>>> always-y += kpatch.lds
>>>>>
>>>>> foo-objs += bar.o
>>>>>
>>>>> all:
>>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
>>>>> clean:
>>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
>>>>> _EOF_
>>>>>
>>>>> Take a look into scripts/Makefile.build:488:
>>>>>
>>>>> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
>>>>>           $(if $(KBUILD_MODULES), $(targets-for-modules)) \
>>>>>           $(subdir-ym) $(always-y)
>>>>>          @:
>>>>>
>>>>> 'always-y' is built after 'targets-for-modules'. This makes
>>>>> 'targets-for-modules' fails because kpatch.lds isn't there.
>>>>
>>>>
>>>> Heh, you rely on the targets built from left to right,
>>>> and you have never thought Make supports the parallel option -j.
>>>
>>> You're right. I missed that.
>>>
>>>>
>>>>
>>>> You need to specify the dependency if you expect objects
>>>> are built in the particular order.
>>>>
>>>> However, in this case, using ldflags-y looks wrong
>>>> in the first place.
>>>>
>>>> The linker script is used when combining the object
>>>> as well as the final link of *.ko
>>
>> We want linker script to be used on both those steps, otherwise modpost
>> fails.
> 
> 
> In that case, does the following work?
> (untested)
> 
> 
> 
> diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
> index e017b17..02d4c66 100644
> --- a/kmod/patch/Makefile
> +++ b/kmod/patch/Makefile
> @@ -12,7 +12,9 @@ endif
> 
>   obj-m += $(KPATCH_NAME).o
>   ldflags-y += -T $(src)/kpatch.lds
> -extra-y := kpatch.lds
> +targets += kpatch.lds
> +
> +$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds
> 
>   $(KPATCH_NAME)-objs += patch-hook.o output.o
> 

Hi Masahiro,

Yeah this is more or less what Artem came up with:
https://github.com/dynup/kpatch/pull/1149

though we hadn't added kpatch.lds to targets.  Is there documentation 
somewhere on what effect "targets" has for out-of-tree builds?

Thanks,

-- Joe


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-16 23:04             ` Joe Lawrence
@ 2020-12-17  8:26               ` Masahiro Yamada
  2020-12-17 11:51                 ` Artem Savkov
  2020-12-17 12:04                 ` Artem Savkov
  0 siblings, 2 replies; 11+ messages in thread
From: Masahiro Yamada @ 2020-12-17  8:26 UTC (permalink / raw)
  To: Joe Lawrence
  Cc: Artem Savkov, WANG Chao, Linux Kernel Mailing List,
	Linux Kbuild mailing list

On Thu, Dec 17, 2020 at 8:04 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
>
> On 12/16/20 1:14 AM, Masahiro Yamada wrote:
> > On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@gmail.com> wrote:
> >>
> >> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
> >>> Sorry for the late reply.
> >>>
> >>> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> >>>> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> >>>>>
> >>>>> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> >>>>>> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> >>>>>>>
> >>>>>>> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> >>>>>>> ("kbuild: do not create built-in objects for external module builds").
> >>>>>>>
> >>>>>>> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> >>>>>>> and 'make M=...' to build livepatch patch module.
> >>>>>>>
> >>>>>>> Add extra-y to targets-for-modules so that such kind of build works
> >>>>>>> properly.
> >>>>>>>
> >>>>>>> Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> >>>>>>> ---
> >>>>>>>   scripts/Makefile.build | 2 +-
> >>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> >>>>>>> index ae647379b579..0113a042d643 100644
> >>>>>>> --- a/scripts/Makefile.build
> >>>>>>> +++ b/scripts/Makefile.build
> >>>>>>> @@ -86,7 +86,7 @@ ifdef need-builtin
> >>>>>>>   targets-for-builtin += $(obj)/built-in.a
> >>>>>>>   endif
> >>>>>>>
> >>>>>>> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> >>>>>>> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> >>>>>>>
> >>>>>>>   ifdef need-modorder
> >>>>>>>   targets-for-modules += $(obj)/modules.order
> >>>>>>> --
> >>>>>>> 2.29.1
> >>>>>>>
> >>>>>>
> >>>>>> NACK.
> >>>>>>
> >>>>>> Please fix your Makefile.
> >>>>>>
> >>>>>> Hint:
> >>>>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> >>>>>>
> >>>>>>
> >>>>>> Probably what you should use is 'targets'.
> >>>>>
> >>>>> I tried with 'targets' and 'always-y'. Both doesn't work for me.
> >>>>>
> >>>>> I narraw it down to the following example:
> >>>>>
> >>>>> cat > Makefile << _EOF_
> >>>>> obj-m += foo.o
> >>>>>
> >>>>> ldflags-y += -T $(src)/kpatch.lds
> >>>>> always-y += kpatch.lds
> >>>>>
> >>>>> foo-objs += bar.o
> >>>>>
> >>>>> all:
> >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> >>>>> clean:
> >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> >>>>> _EOF_
> >>>>>
> >>>>> Take a look into scripts/Makefile.build:488:
> >>>>>
> >>>>> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> >>>>>           $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> >>>>>           $(subdir-ym) $(always-y)
> >>>>>          @:
> >>>>>
> >>>>> 'always-y' is built after 'targets-for-modules'. This makes
> >>>>> 'targets-for-modules' fails because kpatch.lds isn't there.
> >>>>
> >>>>
> >>>> Heh, you rely on the targets built from left to right,
> >>>> and you have never thought Make supports the parallel option -j.
> >>>
> >>> You're right. I missed that.
> >>>
> >>>>
> >>>>
> >>>> You need to specify the dependency if you expect objects
> >>>> are built in the particular order.
> >>>>
> >>>> However, in this case, using ldflags-y looks wrong
> >>>> in the first place.
> >>>>
> >>>> The linker script is used when combining the object
> >>>> as well as the final link of *.ko
> >>
> >> We want linker script to be used on both those steps, otherwise modpost
> >> fails.
> >
> >
> > In that case, does the following work?
> > (untested)
> >
> >
> >
> > diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
> > index e017b17..02d4c66 100644
> > --- a/kmod/patch/Makefile
> > +++ b/kmod/patch/Makefile
> > @@ -12,7 +12,9 @@ endif
> >
> >   obj-m += $(KPATCH_NAME).o
> >   ldflags-y += -T $(src)/kpatch.lds
> > -extra-y := kpatch.lds
> > +targets += kpatch.lds
> > +
> > +$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds
> >
> >   $(KPATCH_NAME)-objs += patch-hook.o output.o
> >
>
> Hi Masahiro,
>
> Yeah this is more or less what Artem came up with:
> https://github.com/dynup/kpatch/pull/1149
>
> though we hadn't added kpatch.lds to targets.  Is there documentation
> somewhere on what effect "targets" has for out-of-tree builds?
>
> Thanks,
>
> -- Joe
>


Please try the rebuild without changing any source code.

If kpatch.lds is needlessly rebuilt, you need to add
it to 'targets'.


In linux-next, this is documented.

Documentation/kbuild/makefile.rst:

        Any target that utilizes if_changed must be listed in $(targets),
        otherwise the command line check will fail, and the target will
        always be built.







-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-17  8:26               ` Masahiro Yamada
@ 2020-12-17 11:51                 ` Artem Savkov
  2020-12-17 12:04                 ` Artem Savkov
  1 sibling, 0 replies; 11+ messages in thread
From: Artem Savkov @ 2020-12-17 11:51 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Joe Lawrence, WANG Chao, Linux Kernel Mailing List,
	Linux Kbuild mailing list

On Thu, Dec 17, 2020 at 05:26:07PM +0900, Masahiro Yamada wrote:
> On Thu, Dec 17, 2020 at 8:04 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
> >
> > On 12/16/20 1:14 AM, Masahiro Yamada wrote:
> > > On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@gmail.com> wrote:
> > >>
> > >> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
> > >>> Sorry for the late reply.
> > >>>
> > >>> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> > >>>> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >>>>>
> > >>>>> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > >>>>>> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >>>>>>>
> > >>>>>>> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > >>>>>>> ("kbuild: do not create built-in objects for external module builds").
> > >>>>>>>
> > >>>>>>> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > >>>>>>> and 'make M=...' to build livepatch patch module.
> > >>>>>>>
> > >>>>>>> Add extra-y to targets-for-modules so that such kind of build works
> > >>>>>>> properly.
> > >>>>>>>
> > >>>>>>> Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > >>>>>>> ---
> > >>>>>>>   scripts/Makefile.build | 2 +-
> > >>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>>
> > >>>>>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > >>>>>>> index ae647379b579..0113a042d643 100644
> > >>>>>>> --- a/scripts/Makefile.build
> > >>>>>>> +++ b/scripts/Makefile.build
> > >>>>>>> @@ -86,7 +86,7 @@ ifdef need-builtin
> > >>>>>>>   targets-for-builtin += $(obj)/built-in.a
> > >>>>>>>   endif
> > >>>>>>>
> > >>>>>>> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > >>>>>>> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > >>>>>>>
> > >>>>>>>   ifdef need-modorder
> > >>>>>>>   targets-for-modules += $(obj)/modules.order
> > >>>>>>> --
> > >>>>>>> 2.29.1
> > >>>>>>>
> > >>>>>>
> > >>>>>> NACK.
> > >>>>>>
> > >>>>>> Please fix your Makefile.
> > >>>>>>
> > >>>>>> Hint:
> > >>>>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> > >>>>>>
> > >>>>>>
> > >>>>>> Probably what you should use is 'targets'.
> > >>>>>
> > >>>>> I tried with 'targets' and 'always-y'. Both doesn't work for me.
> > >>>>>
> > >>>>> I narraw it down to the following example:
> > >>>>>
> > >>>>> cat > Makefile << _EOF_
> > >>>>> obj-m += foo.o
> > >>>>>
> > >>>>> ldflags-y += -T $(src)/kpatch.lds
> > >>>>> always-y += kpatch.lds
> > >>>>>
> > >>>>> foo-objs += bar.o
> > >>>>>
> > >>>>> all:
> > >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> > >>>>> clean:
> > >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> > >>>>> _EOF_
> > >>>>>
> > >>>>> Take a look into scripts/Makefile.build:488:
> > >>>>>
> > >>>>> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> > >>>>>           $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> > >>>>>           $(subdir-ym) $(always-y)
> > >>>>>          @:
> > >>>>>
> > >>>>> 'always-y' is built after 'targets-for-modules'. This makes
> > >>>>> 'targets-for-modules' fails because kpatch.lds isn't there.
> > >>>>
> > >>>>
> > >>>> Heh, you rely on the targets built from left to right,
> > >>>> and you have never thought Make supports the parallel option -j.
> > >>>
> > >>> You're right. I missed that.
> > >>>
> > >>>>
> > >>>>
> > >>>> You need to specify the dependency if you expect objects
> > >>>> are built in the particular order.
> > >>>>
> > >>>> However, in this case, using ldflags-y looks wrong
> > >>>> in the first place.
> > >>>>
> > >>>> The linker script is used when combining the object
> > >>>> as well as the final link of *.ko
> > >>
> > >> We want linker script to be used on both those steps, otherwise modpost
> > >> fails.
> > >
> > >
> > > In that case, does the following work?
> > > (untested)
> > >
> > >
> > >
> > > diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
> > > index e017b17..02d4c66 100644
> > > --- a/kmod/patch/Makefile
> > > +++ b/kmod/patch/Makefile
> > > @@ -12,7 +12,9 @@ endif
> > >
> > >   obj-m += $(KPATCH_NAME).o
> > >   ldflags-y += -T $(src)/kpatch.lds
> > > -extra-y := kpatch.lds
> > > +targets += kpatch.lds
> > > +
> > > +$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds
> > >
> > >   $(KPATCH_NAME)-objs += patch-hook.o output.o
> > >
> >
> > Hi Masahiro,
> >
> > Yeah this is more or less what Artem came up with:
> > https://github.com/dynup/kpatch/pull/1149
> >
> > though we hadn't added kpatch.lds to targets.  Is there documentation
> > somewhere on what effect "targets" has for out-of-tree builds?
> >
> > Thanks,
> >
> > -- Joe
> >
> 
> 
> Please try the rebuild without changing any source code.
> 
> If kpatch.lds is needlessly rebuilt, you need to add
> it to 'targets'.
> 
> 
> In linux-next, this is documented.
> 
> Documentation/kbuild/makefile.rst:
> 
>         Any target that utilizes if_changed must be listed in $(targets),
>         otherwise the command line check will fail, and the target will
>         always be built.

Ah yes, it did rebuild every time. Thank you for the pointer.

-- 
 Artem


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kbuild: add extra-y to targets-for-modules
  2020-12-17  8:26               ` Masahiro Yamada
  2020-12-17 11:51                 ` Artem Savkov
@ 2020-12-17 12:04                 ` Artem Savkov
  1 sibling, 0 replies; 11+ messages in thread
From: Artem Savkov @ 2020-12-17 12:04 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Joe Lawrence, WANG Chao, Linux Kernel Mailing List,
	Linux Kbuild mailing list

On Thu, Dec 17, 2020 at 05:26:07PM +0900, Masahiro Yamada wrote:
> On Thu, Dec 17, 2020 at 8:04 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
> >
> > On 12/16/20 1:14 AM, Masahiro Yamada wrote:
> > > On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@gmail.com> wrote:
> > >>
> > >> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
> > >>> Sorry for the late reply.
> > >>>
> > >>> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
> > >>>> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >>>>>
> > >>>>> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
> > >>>>>> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@ucloud.cn> wrote:
> > >>>>>>>
> > >>>>>>> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
> > >>>>>>> ("kbuild: do not create built-in objects for external module builds").
> > >>>>>>>
> > >>>>>>> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
> > >>>>>>> and 'make M=...' to build livepatch patch module.
> > >>>>>>>
> > >>>>>>> Add extra-y to targets-for-modules so that such kind of build works
> > >>>>>>> properly.
> > >>>>>>>
> > >>>>>>> Signed-off-by: WANG Chao <chao.wang@ucloud.cn>
> > >>>>>>> ---
> > >>>>>>>   scripts/Makefile.build | 2 +-
> > >>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>>
> > >>>>>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > >>>>>>> index ae647379b579..0113a042d643 100644
> > >>>>>>> --- a/scripts/Makefile.build
> > >>>>>>> +++ b/scripts/Makefile.build
> > >>>>>>> @@ -86,7 +86,7 @@ ifdef need-builtin
> > >>>>>>>   targets-for-builtin += $(obj)/built-in.a
> > >>>>>>>   endif
> > >>>>>>>
> > >>>>>>> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > >>>>>>> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> > >>>>>>>
> > >>>>>>>   ifdef need-modorder
> > >>>>>>>   targets-for-modules += $(obj)/modules.order
> > >>>>>>> --
> > >>>>>>> 2.29.1
> > >>>>>>>
> > >>>>>>
> > >>>>>> NACK.
> > >>>>>>
> > >>>>>> Please fix your Makefile.
> > >>>>>>
> > >>>>>> Hint:
> > >>>>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
> > >>>>>>
> > >>>>>>
> > >>>>>> Probably what you should use is 'targets'.
> > >>>>>
> > >>>>> I tried with 'targets' and 'always-y'. Both doesn't work for me.
> > >>>>>
> > >>>>> I narraw it down to the following example:
> > >>>>>
> > >>>>> cat > Makefile << _EOF_
> > >>>>> obj-m += foo.o
> > >>>>>
> > >>>>> ldflags-y += -T $(src)/kpatch.lds
> > >>>>> always-y += kpatch.lds
> > >>>>>
> > >>>>> foo-objs += bar.o
> > >>>>>
> > >>>>> all:
> > >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
> > >>>>> clean:
> > >>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> > >>>>> _EOF_
> > >>>>>
> > >>>>> Take a look into scripts/Makefile.build:488:
> > >>>>>
> > >>>>> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
> > >>>>>           $(if $(KBUILD_MODULES), $(targets-for-modules)) \
> > >>>>>           $(subdir-ym) $(always-y)
> > >>>>>          @:
> > >>>>>
> > >>>>> 'always-y' is built after 'targets-for-modules'. This makes
> > >>>>> 'targets-for-modules' fails because kpatch.lds isn't there.
> > >>>>
> > >>>>
> > >>>> Heh, you rely on the targets built from left to right,
> > >>>> and you have never thought Make supports the parallel option -j.
> > >>>
> > >>> You're right. I missed that.
> > >>>
> > >>>>
> > >>>>
> > >>>> You need to specify the dependency if you expect objects
> > >>>> are built in the particular order.
> > >>>>
> > >>>> However, in this case, using ldflags-y looks wrong
> > >>>> in the first place.
> > >>>>
> > >>>> The linker script is used when combining the object
> > >>>> as well as the final link of *.ko
> > >>
> > >> We want linker script to be used on both those steps, otherwise modpost
> > >> fails.
> > >
> > >
> > > In that case, does the following work?
> > > (untested)
> > >
> > >
> > >
> > > diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
> > > index e017b17..02d4c66 100644
> > > --- a/kmod/patch/Makefile
> > > +++ b/kmod/patch/Makefile
> > > @@ -12,7 +12,9 @@ endif
> > >
> > >   obj-m += $(KPATCH_NAME).o
> > >   ldflags-y += -T $(src)/kpatch.lds
> > > -extra-y := kpatch.lds
> > > +targets += kpatch.lds
> > > +
> > > +$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds
> > >
> > >   $(KPATCH_NAME)-objs += patch-hook.o output.o
> > >
> >
> > Hi Masahiro,
> >
> > Yeah this is more or less what Artem came up with:
> > https://github.com/dynup/kpatch/pull/1149
> >
> > though we hadn't added kpatch.lds to targets.  Is there documentation
> > somewhere on what effect "targets" has for out-of-tree builds?
> >
> > Thanks,
> >
> > -- Joe
> >
> 
> 
> Please try the rebuild without changing any source code.
> 
> If kpatch.lds is needlessly rebuilt, you need to add
> it to 'targets'.
> 
> 
> In linux-next, this is documented.
> 
> Documentation/kbuild/makefile.rst:
> 
>         Any target that utilizes if_changed must be listed in $(targets),
>         otherwise the command line check will fail, and the target will
>         always be built.

Ah yes, it did rebuild every time. Thank you for the pointer.

-- 
 Artem

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-01-17 13:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03  5:44 [PATCH] kbuild: add extra-y to targets-for-modules WANG Chao
2020-11-23  5:23 ` Masahiro Yamada
2020-11-23 15:04   ` WANG Chao
2020-11-25 13:42     ` Masahiro Yamada
2020-12-08  9:20       ` WANG Chao
2020-12-08 14:31         ` Artem Savkov
2020-12-16  6:14           ` Masahiro Yamada
2020-12-16 23:04             ` Joe Lawrence
2020-12-17  8:26               ` Masahiro Yamada
2020-12-17 11:51                 ` Artem Savkov
2020-12-17 12:04                 ` Artem Savkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.