LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] samples: build fix
@ 2008-02-14  3:45 Roland McGrath
  2008-02-14 13:27 ` Mathieu Desnoyers
  0 siblings, 1 reply; 9+ messages in thread
From: Roland McGrath @ 2008-02-14  3:45 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: Mathieu Desnoyers, linux-kernel


The samples/ subdirectory contains only modules.
But the only make run done there is in commands for vmlinux.
I can't see why this was ever done in this nonstandard fashion.
As things stand, the modules don't get built by 'make modules'.

I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
because there is no other conditional like that in the top-level Makefile
and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
always to be included.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 Makefile |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c162370..9e9ce33 100644
--- a/Makefile
+++ b/Makefile
@@ -602,7 +602,7 @@ export mod_strip_cmd
 
 
 ifeq ($(KBUILD_EXTMOD),)
-core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/
+core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/ samples/
 
 vmlinux-dirs	:= $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
 		     $(core-y) $(core-m) $(drivers-y) $(drivers-m) \
@@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
 ifdef CONFIG_HEADERS_CHECK
 	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
 endif
-ifdef CONFIG_SAMPLES
-	$(Q)$(MAKE) $(build)=samples
-endif
 	$(call vmlinux-modpost)
 	$(call if_changed_rule,vmlinux__)
 	$(Q)rm -f .old_version

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

* Re: [PATCH] samples: build fix
  2008-02-14  3:45 [PATCH] samples: build fix Roland McGrath
@ 2008-02-14 13:27 ` Mathieu Desnoyers
  2008-02-14 19:26   ` Roland McGrath
  2008-02-14 19:46   ` Sam Ravnborg
  0 siblings, 2 replies; 9+ messages in thread
From: Mathieu Desnoyers @ 2008-02-14 13:27 UTC (permalink / raw)
  To: Sam Ravnborg, Roland McGrath; +Cc: Linus Torvalds, Andrew Morton, linux-kernel

* Roland McGrath (roland@redhat.com) wrote:
> 
> The samples/ subdirectory contains only modules.
> But the only make run done there is in commands for vmlinux.
> I can't see why this was ever done in this nonstandard fashion.
> As things stand, the modules don't get built by 'make modules'.
> 
> I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> because there is no other conditional like that in the top-level Makefile
> and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> always to be included.
> 

Sam, is this ok with the samples intent ? I think as long as we do not
include them with the kernel image and have a "make samples" to build
them, it's ok. Having them built upon make modules seems like a good
idea to me.

Mathieu

> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
>  Makefile |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c162370..9e9ce33 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -602,7 +602,7 @@ export mod_strip_cmd
>  
>  
>  ifeq ($(KBUILD_EXTMOD),)
> -core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/
> +core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/ samples/
>  
>  vmlinux-dirs	:= $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
>  		     $(core-y) $(core-m) $(drivers-y) $(drivers-m) \
> @@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
>  ifdef CONFIG_HEADERS_CHECK
>  	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
>  endif
> -ifdef CONFIG_SAMPLES
> -	$(Q)$(MAKE) $(build)=samples
> -endif
>  	$(call vmlinux-modpost)
>  	$(call if_changed_rule,vmlinux__)
>  	$(Q)rm -f .old_version

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

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

* Re: [PATCH] samples: build fix
  2008-02-14 13:27 ` Mathieu Desnoyers
@ 2008-02-14 19:26   ` Roland McGrath
  2008-02-14 19:46   ` Sam Ravnborg
  1 sibling, 0 replies; 9+ messages in thread
From: Roland McGrath @ 2008-02-14 19:26 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Sam Ravnborg, Linus Torvalds, Andrew Morton, linux-kernel

> Sam, is this ok with the samples intent ? I think as long as we do not
> include them with the kernel image and have a "make samples" to build
> them, it's ok. Having them built upon make modules seems like a good
> idea to me.

I"m not sure what you mean by "with the kernel image".  
AFAICT, everything under samples/ can only ever be built as modules.
In no event would it affect the vmlinux link.


Thanks,
Roland

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

* Re: [PATCH] samples: build fix
  2008-02-14 13:27 ` Mathieu Desnoyers
  2008-02-14 19:26   ` Roland McGrath
@ 2008-02-14 19:46   ` Sam Ravnborg
  2008-02-14 20:02     ` Roland McGrath
  2008-02-14 20:04     ` Mathieu Desnoyers
  1 sibling, 2 replies; 9+ messages in thread
From: Sam Ravnborg @ 2008-02-14 19:46 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Roland McGrath, Linus Torvalds, Andrew Morton, linux-kernel

On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> * Roland McGrath (roland@redhat.com) wrote:
> > 
> > The samples/ subdirectory contains only modules.
> > But the only make run done there is in commands for vmlinux.
> > I can't see why this was ever done in this nonstandard fashion.
> > As things stand, the modules don't get built by 'make modules'.
> > 
> > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > because there is no other conditional like that in the top-level Makefile
> > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > always to be included.
> > 
> 
> Sam, is this ok with the samples intent ? I think as long as we do not
> include them with the kernel image and have a "make samples" to build
> them, it's ok. Having them built upon make modules seems like a good
> idea to me.

The samples code are supposed to be what the name says 'samples'.
This is not code that are supposed to be part of the built-in kernel.
This is not modules that are supposed to be installes when 
installing modules.

Adding it to core-y as Roland does in the patch below is plain
wrong as it enabled both points above.
The fact that the present code in samples/ does not do this is
in this respect irellevant.

Do we have problems when to build the sampls - then lets
address this issue but not by trying to upgrade the samples
to first class citizen in the kernel - they are not that
and should not be handled like that.

	Sam

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

* Re: [PATCH] samples: build fix
  2008-02-14 19:46   ` Sam Ravnborg
@ 2008-02-14 20:02     ` Roland McGrath
  2008-02-14 20:09       ` Randy Dunlap
  2008-02-14 20:42       ` Sam Ravnborg
  2008-02-14 20:04     ` Mathieu Desnoyers
  1 sibling, 2 replies; 9+ messages in thread
From: Roland McGrath @ 2008-02-14 20:02 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Mathieu Desnoyers, Linus Torvalds, Andrew Morton, linux-kernel

I don't have any axe to grind here, I just could not figure out how to get
the samples/ modules built (without resorting to make M=.../samples/foo and
clobbering my source directory).  If there is another plan, point me to its
documentation.  I can't see the point of the Kconfig option for samples at all.
What does it do?

Because they are only ever modules, the samples are never built in.  I
honestly don't see why the modules shouldn't be installed like any others.
People who are really going to install the kernel for production are not
going to set this Kconfig option.


Thanks,
Roland

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

* Re: [PATCH] samples: build fix
  2008-02-14 19:46   ` Sam Ravnborg
  2008-02-14 20:02     ` Roland McGrath
@ 2008-02-14 20:04     ` Mathieu Desnoyers
  2008-02-14 21:06       ` Sam Ravnborg
  1 sibling, 1 reply; 9+ messages in thread
From: Mathieu Desnoyers @ 2008-02-14 20:04 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Roland McGrath, Linus Torvalds, Andrew Morton, linux-kernel

* Sam Ravnborg (sam@ravnborg.org) wrote:
> On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> > * Roland McGrath (roland@redhat.com) wrote:
> > > 
> > > The samples/ subdirectory contains only modules.
> > > But the only make run done there is in commands for vmlinux.
> > > I can't see why this was ever done in this nonstandard fashion.
> > > As things stand, the modules don't get built by 'make modules'.
> > > 
> > > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > > because there is no other conditional like that in the top-level Makefile
> > > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > > always to be included.
> > > 
> > 
> > Sam, is this ok with the samples intent ? I think as long as we do not
> > include them with the kernel image and have a "make samples" to build
> > them, it's ok. Having them built upon make modules seems like a good
> > idea to me.
> 
> The samples code are supposed to be what the name says 'samples'.
> This is not code that are supposed to be part of the built-in kernel.
> This is not modules that are supposed to be installes when 
> installing modules.
> 
> Adding it to core-y as Roland does in the patch below is plain
> wrong as it enabled both points above.
> The fact that the present code in samples/ does not do this is
> in this respect irellevant.
> 
> Do we have problems when to build the sampls - then lets
> address this issue but not by trying to upgrade the samples
> to first class citizen in the kernel - they are not that
> and should not be handled like that.
> 

Then is there some other way to have the samples built upon "make
modules" that would not install them with other modules ?

Mathieu

> 	Sam

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

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

* Re: [PATCH] samples: build fix
  2008-02-14 20:02     ` Roland McGrath
@ 2008-02-14 20:09       ` Randy Dunlap
  2008-02-14 20:42       ` Sam Ravnborg
  1 sibling, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2008-02-14 20:09 UTC (permalink / raw)
  To: Roland McGrath
  Cc: Sam Ravnborg, Mathieu Desnoyers, Linus Torvalds, Andrew Morton,
	linux-kernel

On Thu, 14 Feb 2008 12:02:57 -0800 (PST) Roland McGrath wrote:

> I don't have any axe to grind here, I just could not figure out how to get
> the samples/ modules built (without resorting to make M=.../samples/foo and
> clobbering my source directory).  If there is another plan, point me to its
> documentation.  I can't see the point of the Kconfig option for samples at all.
> What does it do?
> 
> Because they are only ever modules, the samples are never built in.  I
> honestly don't see why the modules shouldn't be installed like any others.
> People who are really going to install the kernel for production are not
> going to set this Kconfig option.

There used to be a make target called "samples_check" (I could have
misspelled it).  Where did it go?  Sam?

---
~Randy

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

* Re: [PATCH] samples: build fix
  2008-02-14 20:02     ` Roland McGrath
  2008-02-14 20:09       ` Randy Dunlap
@ 2008-02-14 20:42       ` Sam Ravnborg
  1 sibling, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2008-02-14 20:42 UTC (permalink / raw)
  To: Roland McGrath
  Cc: Mathieu Desnoyers, Linus Torvalds, Andrew Morton, linux-kernel

On Thu, Feb 14, 2008 at 12:02:57PM -0800, Roland McGrath wrote:
> I don't have any axe to grind here, I just could not figure out how to get
> the samples/ modules built (without resorting to make M=.../samples/foo and
> clobbering my source directory).

> If there is another plan, point me to its documentation.  

I just double checked and the usual:
make O=output samples/

It works as expected and build all the samples.
It is documented by make help like this:
$ make help
  ...
  dir/            - Build all files in dir and below


The above is generic functionality and I do not see any
need for a special target to build the samples.

> I can't see the point of the Kconfig option for samples at all.
> What does it do?
The samples were added so we avoided them to bit-rot.
We do not want to build samples unconditionally - that should be pretty
obvious.
So maybe I did not get what you actually was asking?

> Because they are only ever modules, the samples are never built in.
There is _nothing_ preventing us from adding stuff tomorrow that has
be build as built-in. Some sample code documenting early cpu init
stuff for instance. And we do not want this code to clash with
other kernel code if global symbols are duplicated.

This thinking that samples are solely modules are wrong. It is correct that
most of it is expected to be modules but not all of it.

I only found one little glitch.
$ make modules
does not build the samples.

	Sam

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

* Re: [PATCH] samples: build fix
  2008-02-14 20:04     ` Mathieu Desnoyers
@ 2008-02-14 21:06       ` Sam Ravnborg
  0 siblings, 0 replies; 9+ messages in thread
From: Sam Ravnborg @ 2008-02-14 21:06 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Roland McGrath, Linus Torvalds, Andrew Morton, linux-kernel

On Thu, Feb 14, 2008 at 03:04:04PM -0500, Mathieu Desnoyers wrote:
> * Sam Ravnborg (sam@ravnborg.org) wrote:
> > On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> > > * Roland McGrath (roland@redhat.com) wrote:
> > > > 
> > > > The samples/ subdirectory contains only modules.
> > > > But the only make run done there is in commands for vmlinux.
> > > > I can't see why this was ever done in this nonstandard fashion.
> > > > As things stand, the modules don't get built by 'make modules'.
> > > > 
> > > > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > > > because there is no other conditional like that in the top-level Makefile
> > > > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > > > always to be included.
> > > > 
> > > 
> > > Sam, is this ok with the samples intent ? I think as long as we do not
> > > include them with the kernel image and have a "make samples" to build
> > > them, it's ok. Having them built upon make modules seems like a good
> > > idea to me.
> > 
> > The samples code are supposed to be what the name says 'samples'.
> > This is not code that are supposed to be part of the built-in kernel.
> > This is not modules that are supposed to be installes when 
> > installing modules.
> > 
> > Adding it to core-y as Roland does in the patch below is plain
> > wrong as it enabled both points above.
> > The fact that the present code in samples/ does not do this is
> > in this respect irellevant.
> > 
> > Do we have problems when to build the sampls - then lets
> > address this issue but not by trying to upgrade the samples
> > to first class citizen in the kernel - they are not that
> > and should not be handled like that.
> > 
> 
> Then is there some other way to have the samples built upon "make
> modules" that would not install them with other modules ?
The simple solution is like attached.
I do not care if they are build as part of vmlinux or modules build.
So if it is generally preferred we could move it like this.

Comments?

	Sam

diff --git a/Makefile b/Makefile
index c162370..c72afa9 100644
--- a/Makefile
+++ b/Makefile
@@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
 ifdef CONFIG_HEADERS_CHECK
 	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
 endif
-ifdef CONFIG_SAMPLES
-	$(Q)$(MAKE) $(build)=samples
-endif
 	$(call vmlinux-modpost)
 	$(call if_changed_rule,vmlinux__)
 	$(Q)rm -f .old_version
@@ -1036,6 +1033,9 @@ all: modules
 PHONY += modules
 modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux)
 	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
+ifdef CONFIG_SAMPLES
+	$(Q)$(MAKE) $(build)=samples
+endif
 	@echo '  Building modules, stage 2.';
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
 

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

end of thread, other threads:[~2008-02-14 21:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-14  3:45 [PATCH] samples: build fix Roland McGrath
2008-02-14 13:27 ` Mathieu Desnoyers
2008-02-14 19:26   ` Roland McGrath
2008-02-14 19:46   ` Sam Ravnborg
2008-02-14 20:02     ` Roland McGrath
2008-02-14 20:09       ` Randy Dunlap
2008-02-14 20:42       ` Sam Ravnborg
2008-02-14 20:04     ` Mathieu Desnoyers
2008-02-14 21:06       ` Sam Ravnborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).