Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Nick Piggin]

On Thursday 06 March 2008 09:38, Christoph Lameter wrote:
> Remove aliases of PG_xxx. We can easily drop those now and alias by
> specifying the PG_xxx flag in the macro that generates the functions.
>
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
>
> ---
>  include/linux/page-flags.h |   10 +++-------
>  mm/page_alloc.c            |    2 +-
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> Index: linux-2.6.25-rc3-mm1/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.25-rc3-mm1.orig/mm/page_alloc.c	2008-03-05 14:17:09.963838055
> -0800 +++ linux-2.6.25-rc3-mm1/mm/page_alloc.c	2008-03-05
> 14:21:45.372755277 -0800 @@ -650,7 +650,7 @@ static int
> prep_new_page(struct page *pa
>  	if (PageReserved(page))
>  		return 1;
>
> -	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_readahead |
> +	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_reclaim |
>  			1 << PG_referenced | 1 << PG_arch_1 |
>  			1 << PG_owner_priv_1 | 1 << PG_mappedtodisk);
>  	set_page_private(page, 0);
> Index: linux-2.6.25-rc3-mm1/include/linux/page-flags.h
> ===================================================================
> --- linux-2.6.25-rc3-mm1.orig/include/linux/page-flags.h	2008-03-05
> 14:21:29.689386158 -0800 +++
> linux-2.6.25-rc3-mm1/include/linux/page-flags.h	2008-03-05
> 14:21:45.372755277 -0800 @@ -77,8 +77,6 @@ enum pageflags {
>  	PG_active,
>  	PG_slab,
>  	PG_owner_priv_1,	/* Owner use. If pagecache, fs may use*/
> -	PG_checked = PG_owner_priv_1, /* Used by some filesystems */
> -	PG_pinned = PG_owner_priv_1, /* Xen pinned pagetable */
>  	PG_arch_1,
>  	PG_reserved,
>  	PG_private,		/* If pagecache, has fs-private data */
> @@ -87,8 +85,6 @@ enum pageflags {
>  	PG_swapcache,		/* Swap page: swp_entry_t in private */
>  	PG_mappedtodisk,	/* Has blocks allocated on-disk */
>  	PG_reclaim,		/* To be reclaimed asap */
> -	/* PG_readahead is only used for file reads; PG_reclaim is only for
> writes */ -	PG_readahead = PG_reclaim, /* Reminder to do async read-ahead
> */ PG_buddy,		/* Page is free, on buddy lists */

IMO it's nice to see these alias up front.

Otherwise the patchset looks pretty good, nice work. I actually hate
macros for generating things, but I try to pick my fights ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Christoph Lameter]

On Thu, 6 Mar 2008, Nick Piggin wrote:

> >  	PG_mappedtodisk,	/* Has blocks allocated on-disk */
> >  	PG_reclaim,		/* To be reclaimed asap */
> > -	/* PG_readahead is only used for file reads; PG_reclaim is only for
> > writes */ -	PG_readahead = PG_reclaim, /* Reminder to do async read-ahead
> > */ PG_buddy,		/* Page is free, on buddy lists */
> 
> IMO it's nice to see these alias up front.

I could add a comment pointing to the aliases for those that are aliases?



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Nick Piggin]

On Friday 07 March 2008 09:51, Christoph Lameter wrote:
> On Thu, 6 Mar 2008, Nick Piggin wrote:
> > >  	PG_mappedtodisk,	/* Has blocks allocated on-disk */
> > >  	PG_reclaim,		/* To be reclaimed asap */
> > > -	/* PG_readahead is only used for file reads; PG_reclaim is only for
> > > writes */ -	PG_readahead = PG_reclaim, /* Reminder to do async
> > > read-ahead */ PG_buddy,		/* Page is free, on buddy lists */
> >
> > IMO it's nice to see these alias up front.
>
> I could add a comment pointing to the aliases for those that are aliases?

Yeah that would be better than nothing. I didn't quite 
understand why you made this change in the first place
though.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Christoph Lameter]

On Fri, 7 Mar 2008, Nick Piggin wrote:

> On Friday 07 March 2008 09:51, Christoph Lameter wrote:
> > On Thu, 6 Mar 2008, Nick Piggin wrote:
> > > >  	PG_mappedtodisk,	/* Has blocks allocated on-disk */
> > > >  	PG_reclaim,		/* To be reclaimed asap */
> > > > -	/* PG_readahead is only used for file reads; PG_reclaim is only for
> > > > writes */ -	PG_readahead = PG_reclaim, /* Reminder to do async
> > > > read-ahead */ PG_buddy,		/* Page is free, on buddy lists */
> > >
> > > IMO it's nice to see these alias up front.
> >
> > I could add a comment pointing to the aliases for those that are aliases?
> 
> Yeah that would be better than nothing. I didn't quite 
> understand why you made this change in the first place
> though.

It avoids us having to deal with aliases in the future. PG_xx at this 
point is not unique which can be confusing. See the PG_reclaim in 
mm/page_alloc.c. It also means PG_readahead. If I look for 
handling of PG_readahead then I wont find it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Nick Piggin]

On Friday 07 March 2008 12:38, Christoph Lameter wrote:
> On Fri, 7 Mar 2008, Nick Piggin wrote:
> > On Friday 07 March 2008 09:51, Christoph Lameter wrote:
> > > On Thu, 6 Mar 2008, Nick Piggin wrote:
> > > > >  	PG_mappedtodisk,	/* Has blocks allocated on-disk */
> > > > >  	PG_reclaim,		/* To be reclaimed asap */
> > > > > -	/* PG_readahead is only used for file reads; PG_reclaim is only
> > > > > for writes */ -	PG_readahead = PG_reclaim, /* Reminder to do async
> > > > > read-ahead */ PG_buddy,		/* Page is free, on buddy lists */
> > > >
> > > > IMO it's nice to see these alias up front.
> > >
> > > I could add a comment pointing to the aliases for those that are
> > > aliases?
> >
> > Yeah that would be better than nothing. I didn't quite
> > understand why you made this change in the first place
> > though.
>
> It avoids us having to deal with aliases in the future.

It doesn't. You still have to deal with them.


> PG_xx at this 
> point is not unique which can be confusing. See the PG_reclaim in
> mm/page_alloc.c. It also means PG_readahead. If I look for
> handling of PG_readahead then I wont find it.

You can't just pretend not to deal with aliases at that point
in mm/page_alloc.c just becuase you only have one name for the
bit position.

You still have to know that checking for PG_reclaim in bad_page
can only be done if it is *also* a bug for PG_readahead to be
found set at that point too. Because it is an alias.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Christoph Lameter]

On Fri, 7 Mar 2008, Nick Piggin wrote:

> > It avoids us having to deal with aliases in the future.
> 
> It doesn't. You still have to deal with them.

Sortof.

You do not have to deal with it on the level of the PG_xxx enum constant. 
Yes you will have to deal with the aliases at the level of the 
functions.

> > PG_xx at this 
> > point is not unique which can be confusing. See the PG_reclaim in
> > mm/page_alloc.c. It also means PG_readahead. If I look for
> > handling of PG_readahead then I wont find it.
> 
> You can't just pretend not to deal with aliases at that point
> in mm/page_alloc.c just becuase you only have one name for the
> bit position.

If you only have one name for the bit position the you can localize the 
aliases and uses of that bit. This means you can go from a bit that you 
see set while debugging to the PG_xxx flag and then look for uses. Which 
will turn up aliases.

> You still have to know that checking for PG_reclaim in bad_page
> can only be done if it is *also* a bug for PG_readahead to be
> found set at that point too. Because it is an alias.

True.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 8/8] Pageflags: Eliminate PG_xxx aliases

[Nick Piggin]

On Friday 07 March 2008 14:53, Christoph Lameter wrote:
> On Fri, 7 Mar 2008, Nick Piggin wrote:
> > > It avoids us having to deal with aliases in the future.
> >
> > It doesn't. You still have to deal with them.
>
> Sortof.
>
> You do not have to deal with it on the level of the PG_xxx enum constant.

But that's the easy part, and the part that I think is actually
useful because you get to explicitly see the aliases.


> Yes you will have to deal with the aliases at the level of the
> functions.

Which is the hard part.


> > > PG_xx at this
> > > point is not unique which can be confusing. See the PG_reclaim in
> > > mm/page_alloc.c. It also means PG_readahead. If I look for
> > > handling of PG_readahead then I wont find it.
> >
> > You can't just pretend not to deal with aliases at that point
> > in mm/page_alloc.c just becuase you only have one name for the
> > bit position.
>
> If you only have one name for the bit position the you can localize the
> aliases and uses of that bit. This means you can go from a bit that you
> see set while debugging to the PG_xxx flag and then look for uses. Which
> will turn up aliases.

I don't understand how this would be any different from the current
code except with the curent code, you only have to look for aliases
in one place (ie. the PG_ definitions).


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Andrew Morton]

On Wed, 05 Mar 2008 14:38:17 -0800 Christoph Lameter <clameter@sgi.com> wrote:

> The use of enums create constants that are not available to the preprocessor
> when building the kernel (f.e. MAX_NR_ZONES).
> 
> Arch code already has a way to export constants calculated to the
> preprocessor through the asm-offsets.c file. Generate something
> similar for the core kernel through kbuild.
> 

err, this patch needs help.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/linux/bounds.h	2008-02-29 19:29:50.000000000 -0800
> @@ -0,0 +1,10 @@
> +#ifndef __LINUX_BOUNDS_H__
> +#define __LINUX_BOUNDS_H__
> +/*
> + * DO NOT MODIFY.
> + *
> + * This file was generated by Kbuild
> + *
> + */
> +
> +#endif

a) I'm not sure that we should check in a file which is supposed to get
   overwritten at build-time.

b) Is this `make O=' friendly?

c)
	make mrproper
	make allmodconfig
	make

In file included from include/linux/mm.h:192,
                 from include/linux/suspend.h:11,
                 from arch/x86/kernel/asm-offsets_64.c:12,
                 from arch/x86/kernel/asm-offsets.c:4:
include/linux/page-flags.h:10:26: error: linux/bounds.h: No such file or directory

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Christoph Lameter]

On Wed, 5 Mar 2008, Andrew Morton wrote:

> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/include/linux/bounds.h	2008-02-29 19:29:50.000000000 -0800
> > @@ -0,0 +1,10 @@
> > +#ifndef __LINUX_BOUNDS_H__
> > +#define __LINUX_BOUNDS_H__
> > +/*
> > + * DO NOT MODIFY.
> > + *
> > + * This file was generated by Kbuild
> > + *
> > + */
> > +
> > +#endif
> 
> a) I'm not sure that we should check in a file which is supposed to get
>    overwritten at build-time.

Well but it needs to be present in order to generate it. If the 
definitions are not present then the new values of the constants cannot be 
determined. A .config determined the value of these.
 
> b) Is this `make O=' friendly?

These are just symbol definitions. Should be handled the 
same way as arch/x86/*/asm-offsets.* stuff 

> c)
> 	make mrproper
> 	make allmodconfig
> 	make
> 
> In file included from include/linux/mm.h:192,
>                  from include/linux/suspend.h:11,
>                  from arch/x86/kernel/asm-offsets_64.c:12,
>                  from arch/x86/kernel/asm-offsets.c:4:
> include/linux/page-flags.h:10:26: error: linux/bounds.h: No such file or directory

Hmmm... It should not be cleaned....

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Sam Ravnborg]

On Thu, Mar 06, 2008 at 12:20:17PM -0800, Christoph Lameter wrote:
> On Wed, 5 Mar 2008, Andrew Morton wrote:
> 
> > > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > > +++ linux-2.6/include/linux/bounds.h	2008-02-29 19:29:50.000000000 -0800
> > > @@ -0,0 +1,10 @@
> > > +#ifndef __LINUX_BOUNDS_H__
> > > +#define __LINUX_BOUNDS_H__
> > > +/*
> > > + * DO NOT MODIFY.
> > > + *
> > > + * This file was generated by Kbuild
> > > + *
> > > + */
> > > +
> > > +#endif
> > 
> > a) I'm not sure that we should check in a file which is supposed to get
> >    overwritten at build-time.
> 
> Well but it needs to be present in order to generate it. If the 
> definitions are not present then the new values of the constants cannot be 
> determined. A .config determined the value of these.

Ehh - the file above is empty.
I do not understand why we need an empty file to be present???

>  
> > b) Is this `make O=' friendly?
> 
> These are just symbol definitions. Should be handled the 
> same way as arch/x86/*/asm-offsets.* stuff 
> 
> > c)
> > 	make mrproper
> > 	make allmodconfig
> > 	make
> > 
> > In file included from include/linux/mm.h:192,
> >                  from include/linux/suspend.h:11,
> >                  from arch/x86/kernel/asm-offsets_64.c:12,
> >                  from arch/x86/kernel/asm-offsets.c:4:
> > include/linux/page-flags.h:10:26: error: linux/bounds.h: No such file or directory
> 
So asm-offset need bounds.h.
We better tell make so - updated patch below.

	Sam

diff --git a/Kbuild b/Kbuild
index 1570d24..2a52057 100644
--- a/Kbuild
+++ b/Kbuild
@@ -1,19 +1,54 @@
 #
 # Kbuild for top-level directory of the kernel
 # This file takes care of the following:
-# 1) Generate asm-offsets.h
-# 2) Check for missing system calls
+# 1) Generate bounds.h
+# 2) Generate asm-offsets.h (may need bounds.h)
+# 3) Check for missing system calls
 
 #####
-# 1) Generate asm-offsets.h
+# 1) Generate bounds.h
+
+bounds-file := include/linux/bounds.h
+
+always  := $(bounds-file)
+targets := $(bounds-file) kernel/bounds.s
+
+quiet_cmd_bounds = GEN     $@
+define cmd_bounds
+	(set -e; \
+	 echo "#ifndef __LINUX_BOUNDS_H__"; \
+	 echo "#define __LINUX_BOUNDS_H__"; \
+	 echo "/*"; \
+	 echo " * DO NOT MODIFY."; \
+	 echo " *"; \
+	 echo " * This file was generated by Kbuild"; \
+	 echo " *"; \
+	 echo " */"; \
+	 echo ""; \
+	 sed -ne $(sed-y) $<; \
+	 echo ""; \
+	 echo "#endif" ) > $@
+endef
+
+# We use internal kbuild rules to avoid the "is up to date" message from make
+kernel/bounds.s: kernel/bounds.c FORCE
+	$(Q)mkdir -p $(dir $@)
+	$(call if_changed_dep,cc_s_c)
+
+$(obj)/$(bounds-file): kernel/bounds.s Kbuild
+	$(Q)mkdir -p $(dir $@)
+	$(call cmd,bounds)
+
+#####
+# 2) Generate asm-offsets.h
 #
 
 offsets-file := include/asm-$(SRCARCH)/asm-offsets.h
 
-always  := $(offsets-file)
-targets := $(offsets-file)
+always  += $(offsets-file)
+targets += $(offsets-file)
 targets += arch/$(SRCARCH)/kernel/asm-offsets.s
-clean-files := $(addprefix $(objtree)/,$(targets))
+
 
 # Default sed regexp - multiline due to syntax constraints
 define sed-y
@@ -40,7 +75,8 @@ define cmd_offsets
 endef
 
 # We use internal kbuild rules to avoid the "is up to date" message from make
-arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c FORCE
+arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c \
+                                      $(obj)/$(bounds-file) FORCE
 	$(Q)mkdir -p $(dir $@)
 	$(call if_changed_dep,cc_s_c)
 
@@ -49,7 +85,7 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
 	$(call cmd,offsets)
 
 #####
-# 2) Check for missing system calls
+# 3) Check for missing system calls
 #
 
 quiet_cmd_syscalls = CALL    $<
@@ -58,3 +94,7 @@ quiet_cmd_syscalls = CALL    $<
 PHONY += missing-syscalls
 missing-syscalls: scripts/checksyscalls.sh FORCE
 	$(call cmd,syscalls)
+
+# Delete all targets during make clean
+clean-files := $(addprefix $(objtree)/,$(targets))
+

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Christoph Lameter]

On Thu, 6 Mar 2008, Sam Ravnborg wrote:

> Ehh - the file above is empty.
> I do not understand why we need an empty file to be present???

Its going to be populated in later patches of the patchset.

> We better tell make so - updated patch below.

I am very thankful for your help.... Will try that.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Christoph Lameter]

Hmmm.. Even after the Sam's fixes: I still have the recursion probblem 
that include/linux/bounds.h needs to exist and provide some value for 
the constants in order to create kernel/bounds.c. And kernel/bounds.c is 
needed then to create bounds.s which creates bounds.h. Argh!


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Sam Ravnborg]

On Thu, Mar 06, 2008 at 02:43:53PM -0800, Christoph Lameter wrote:
> Hmmm.. Even after the Sam's fixes: I still have the recursion probblem 
> that include/linux/bounds.h needs to exist and provide some value for 
> the constants in order to create kernel/bounds.c. And kernel/bounds.c is 
> needed then to create bounds.s which creates bounds.h. Argh!

I could only come up with following hack. And too late to try more today.

It your test proves that it is OK then:
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>

(But I had preferred the recursive dependency to be gone...)

	Sam

diff --git a/Kbuild b/Kbuild
index 1570d24..37a270f 100644
--- a/Kbuild
+++ b/Kbuild
@@ -2,18 +2,21 @@
 # Kbuild for top-level directory of the kernel
 # This file takes care of the following:
 # 1) Generate asm-offsets.h
-# 2) Check for missing system calls
+# 2) Generate bounds.h
+# 3) Check for missing system calls
 
 #####
 # 1) Generate asm-offsets.h
 #
+# Note: We create a temporary empty bounds.h file to build asm-offsets.s
 
 offsets-file := include/asm-$(SRCARCH)/asm-offsets.h
+bounds-file  := include/linux/bounds.h
 
-always  := $(offsets-file)
+always  := $(bounds-file) $(offsets-file)
 targets := $(offsets-file)
 targets += arch/$(SRCARCH)/kernel/asm-offsets.s
-clean-files := $(addprefix $(objtree)/,$(targets))
+
 
 # Default sed regexp - multiline due to syntax constraints
 define sed-y
@@ -42,6 +45,7 @@ endef
 # We use internal kbuild rules to avoid the "is up to date" message from make
 arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c FORCE
 	$(Q)mkdir -p $(dir $@)
+	$(Q)test -f $(bounds-file) || (mkdir -p $(dir $(bounds-file)) && touch $(bounds-file))
 	$(call if_changed_dep,cc_s_c)
 
 $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
@@ -49,7 +53,39 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
 	$(call cmd,offsets)
 
 #####
-# 2) Check for missing system calls
+# 2) Generate bounds.h
+
+always  += $(bounds-file)
+targets += $(bounds-file) kernel/bounds.s
+
+quiet_cmd_bounds = GEN     $@
+define cmd_bounds
+	(set -e; \
+	 echo "#ifndef __LINUX_BOUNDS_H__"; \
+	 echo "#define __LINUX_BOUNDS_H__"; \
+	 echo "/*"; \
+	 echo " * DO NOT MODIFY."; \
+	 echo " *"; \
+	 echo " * This file was generated by Kbuild"; \
+	 echo " *"; \
+	 echo " */"; \
+	 echo ""; \
+	 sed -ne $(sed-y) $<; \
+	 echo ""; \
+	 echo "#endif" ) > $@
+endef
+
+# We use internal kbuild rules to avoid the "is up to date" message from make
+kernel/bounds.s: kernel/bounds.c $(obj)/$(offsets-file) FORCE
+	$(Q)mkdir -p $(dir $@)
+	$(call if_changed_dep,cc_s_c)
+
+$(obj)/$(bounds-file): kernel/bounds.s Kbuild
+	$(Q)mkdir -p $(dir $@)
+	$(call cmd,bounds)
+
+#####
+# 3) Check for missing system calls
 #
 
 quiet_cmd_syscalls = CALL    $<
@@ -58,3 +94,7 @@ quiet_cmd_syscalls = CALL    $<
 PHONY += missing-syscalls
 missing-syscalls: scripts/checksyscalls.sh FORCE
 	$(call cmd,syscalls)
+
+# Delete all targets during make clean
+clean-files := $(addprefix $(objtree)/,$(targets))
+

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Christoph Lameter]

On Thu, 6 Mar 2008, Sam Ravnborg wrote:

> (But I had preferred the recursive dependency to be gone...)

I suppose we could add an 

#ifndef NR_MAX_ZONES
#define NR_MAX_ZONES 1
#endif

somewheres. Not that elegant though.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/8] Kbuild: Create a way to create preprocessor constants from C expressions

[Sam Ravnborg]

Hi Christoph

Sorry for getting back to you so late. Too busy with day-time job atm.

On Wed, Mar 05, 2008 at 02:38:17PM -0800, Christoph Lameter wrote:
> The use of enums create constants that are not available to the preprocessor
> when building the kernel (f.e. MAX_NR_ZONES).
> 
> Arch code already has a way to export constants calculated to the
> preprocessor through the asm-offsets.c file. Generate something
> similar for the core kernel through kbuild.
> 
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> ---
>  Kbuild                 |   31 +++++++++++++++++++++++++++++--
>  include/linux/bounds.h |   10 ++++++++++
>  kernel/bounds.c        |   16 ++++++++++++++++
>  3 files changed, 55 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/Kbuild
> ===================================================================
> --- linux-2.6.orig/Kbuild	2008-02-29 19:27:40.000000000 -0800
> +++ linux-2.6/Kbuild	2008-02-29 19:29:38.000000000 -0800
> @@ -9,9 +9,10 @@
>  #
>  
>  offsets-file := include/asm-$(SRCARCH)/asm-offsets.h
> +bounds := include/linux/bounds.h
>  
> -always  := $(offsets-file)
> -targets := $(offsets-file)
> +always  := $(offsets-file) $(bounds)
> +targets := $(offsets-file) $(bounds)
>  targets += arch/$(SRCARCH)/kernel/asm-offsets.s
>  clean-files := $(addprefix $(objtree)/,$(targets))

We build a bounds.s file later so it needs to be listed as
we do with asm-offsets.s:

targets += kernel/bounds.s

>  
> @@ -39,6 +40,23 @@ define cmd_offsets
>  	 echo "#endif" ) > $@
>  endef
>  
> +quiet_cmd_bounds = GEN     $@
> +define cmd_bounds
> +	(set -e; \
> +	 echo "#ifndef __LINUX_BOUNDS_H__"; \
> +	 echo "#define __LINUX_BOUNDS_H__"; \
> +	 echo "/*"; \
> +	 echo " * DO NOT MODIFY."; \
> +	 echo " *"; \
> +	 echo " * This file was generated by Kbuild"; \
> +	 echo " *"; \
> +	 echo " */"; \
> +	 echo ""; \
> +	 sed -ne $(sed-y) $<; \
> +	 echo ""; \
> +	 echo "#endif" ) > $@
> +endef
> +
>  # We use internal kbuild rules to avoid the "is up to date" message from make
>  arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c FORCE
>  	$(Q)mkdir -p $(dir $@)
> @@ -48,6 +66,15 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/
>  	$(Q)mkdir -p $(dir $@)
>  	$(call cmd,offsets)
>  
> +# We use internal kbuild rules to avoid the "is up to date" message from make
> +kernel/bounds.s: kernel/bounds.c FORCE
> +	$(Q)mkdir -p $(dir $@)
> +	$(call if_changed_dep,cc_s_c)
OK

> +
> +$(obj)/$(bounds): kernel/bounds.s Kbuild
> +	$(Q)mkdir -p $(dir $@)
> +	$(call cmd,bounds)
OK


> +
>  #####
>  # 2) Check for missing system calls
>  #
> Index: linux-2.6/kernel/bounds.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/kernel/bounds.c	2008-02-29 19:29:38.000000000 -0800
> @@ -0,0 +1,16 @@
> +/*
> + * Generate definitions needed by the preprocessor.
> + * This code generates raw asm output which is post-processed
> + * to extract and format the required data.
> + */
> +
> +#include <linux/mm.h>
> +
> +#define DEFINE(sym, val) \
> +        asm volatile("\n->" #sym " %0 " #val : : "i" (val))
> +
> +#define BLANK() asm volatile("\n->" : : )
> +
> +void foo(void)
> +{
> +}
> Index: linux-2.6/include/linux/bounds.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/include/linux/bounds.h	2008-02-29 19:29:50.000000000 -0800
> @@ -0,0 +1,10 @@
NO NO - we do not add files to the tree we overwrite.
The kbuild stuff will take care that the file is generated before
we start the kernel build.

I have fixed it up and rearranged stuff to follow the style
of the Kbuild file.

diff --git a/Kbuild b/Kbuild
index 1570d24..0198e3f 100644
--- a/Kbuild
+++ b/Kbuild
@@ -2,7 +2,8 @@
 # Kbuild for top-level directory of the kernel
 # This file takes care of the following:
 # 1) Generate asm-offsets.h
-# 2) Check for missing system calls
+# 2) Generate bounds.h
+# 3) Check for missing system calls
 
 #####
 # 1) Generate asm-offsets.h
@@ -13,7 +14,7 @@ offsets-file := include/asm-$(SRCARCH)/asm-offsets.h
 always  := $(offsets-file)
 targets := $(offsets-file)
 targets += arch/$(SRCARCH)/kernel/asm-offsets.s
-clean-files := $(addprefix $(objtree)/,$(targets))
+
 
 # Default sed regexp - multiline due to syntax constraints
 define sed-y
@@ -49,7 +50,41 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
 	$(call cmd,offsets)
 
 #####
-# 2) Check for missing system calls
+# 2) Generate bounds.h
+
+bounds-file := include/linux/bounds.h
+
+always  += $(bounds-file)
+targets += $(bounds-file) kernel/bounds.s
+
+quiet_cmd_bounds = GEN     $@
+define cmd_bounds
+	(set -e; \
+	 echo "#ifndef __LINUX_BOUNDS_H__"; \
+	 echo "#define __LINUX_BOUNDS_H__"; \
+	 echo "/*"; \
+	 echo " * DO NOT MODIFY."; \
+	 echo " *"; \
+	 echo " * This file was generated by Kbuild"; \
+	 echo " *"; \
+	 echo " */"; \
+	 echo ""; \
+	 sed -ne $(sed-y) $<; \
+	 echo ""; \
+	 echo "#endif" ) > $@
+endef
+
+# We use internal kbuild rules to avoid the "is up to date" message from make
+kernel/bounds.s: kernel/bounds.c FORCE
+	$(Q)mkdir -p $(dir $@)
+	$(call if_changed_dep,cc_s_c)
+
+$(obj)/$(bounds-file): kernel/bounds.s Kbuild
+	$(Q)mkdir -p $(dir $@)
+	$(call cmd,bounds)
+
+#####
+# 3) Check for missing system calls
 #
 
 quiet_cmd_syscalls = CALL    $<
@@ -58,3 +93,7 @@ quiet_cmd_syscalls = CALL    $<
 PHONY += missing-syscalls
 missing-syscalls: scripts/checksyscalls.sh FORCE
 	$(call cmd,syscalls)
+
+# Delete all targets during make clean
+clean-files := $(addprefix $(objtree)/,$(targets))
+

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>