2012-08-13 20:57:34

by Thai Bui

[permalink] [raw]
Subject: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

Putting DEBUG_INITCALL config option to compile out the command-line option
"initcall_debug".

Signed-off-by: Thai Bui <[email protected]>
Reviewed-by: Josh Triplett <[email protected]>
---
Documentation/kernel-parameters.txt | 3 ++-
include/linux/init.h | 4 ++++
init/main.c | 2 ++
lib/Kconfig.debug | 9 +++++++++
4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index d99fd9c..3dbaf15 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

initcall_debug [KNL] Trace initcalls as they are executed. Useful
for working out where the kernel is dying during
- startup.
+ startup. DEBUG_INITCALL needs to be enabled in order
+ for this option to work.

initrd= [BOOT] Specify the location of the initial ramdisk

diff --git a/include/linux/init.h b/include/linux/init.h
index 6b95109..d2f31f1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -157,7 +157,11 @@ void prepare_namespace(void);

extern void (*late_time_init)(void);

+#ifdef CONFIG_DEBUG_INITCALL
extern bool initcall_debug;
+#else
+static const bool initcall_debug = false;
+#endif /* CONFIG_DEBUG_INITCALL */

#endif

diff --git a/init/main.c b/init/main.c
index ff49a6d..65837f7 100644
--- a/init/main.c
+++ b/init/main.c
@@ -648,8 +648,10 @@ static void __init do_ctors(void)
#endif
}

+#ifdef CONFIG_DEBUG_INITCALL
bool initcall_debug;
core_param(initcall_debug, initcall_debug, bool, 0644);
+#endif /* CONFIG_DEBUG_INITCALL */

static char msgbuf[64];

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8745ac7..424ac93 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT

If unsure, say N.

+config DEBUG_INITCALL
+ bool "Debug initcalls as they are executed"
+ depends on DEBUG_KERNEL
+ help
+ Enable this for tracing initcalls during startup. Useful for working
+ out where the kernel is dying during startup.
+
+ If unsure, say N
+
config DEBUG_MEMORY_INIT
bool "Debug memory initialisation" if EXPERT
default !EXPERT
--
1.7.9.5


2012-08-13 21:09:25

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On 08/13/2012 01:57 PM, Thai Bui wrote:

> Putting DEBUG_INITCALL config option to compile out the command-line option
> "initcall_debug".

The patch description does not tell us Why.

presumably to save some memory?

How much memory is saved by disabling DEBUG_INITCALL?


> Signed-off-by: Thai Bui <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 3 ++-
> include/linux/init.h | 4 ++++
> init/main.c | 2 ++
> lib/Kconfig.debug | 9 +++++++++
> 4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index d99fd9c..3dbaf15 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> initcall_debug [KNL] Trace initcalls as they are executed. Useful
> for working out where the kernel is dying during
> - startup.
> + startup. DEBUG_INITCALL needs to be enabled in order
> + for this option to work.
>
> initrd= [BOOT] Specify the location of the initial ramdisk
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 6b95109..d2f31f1 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -157,7 +157,11 @@ void prepare_namespace(void);
>
> extern void (*late_time_init)(void);
>
> +#ifdef CONFIG_DEBUG_INITCALL
> extern bool initcall_debug;
> +#else
> +static const bool initcall_debug = false;
> +#endif /* CONFIG_DEBUG_INITCALL */
>
> #endif
>
> diff --git a/init/main.c b/init/main.c
> index ff49a6d..65837f7 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -648,8 +648,10 @@ static void __init do_ctors(void)
> #endif
> }
>
> +#ifdef CONFIG_DEBUG_INITCALL
> bool initcall_debug;
> core_param(initcall_debug, initcall_debug, bool, 0644);
> +#endif /* CONFIG_DEBUG_INITCALL */
>
> static char msgbuf[64];
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8745ac7..424ac93 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT
>
> If unsure, say N.
>
> +config DEBUG_INITCALL
> + bool "Debug initcalls as they are executed"
> + depends on DEBUG_KERNEL
> + help
> + Enable this for tracing initcalls during startup. Useful for working
> + out where the kernel is dying during startup.
> +
> + If unsure, say N
> +
> config DEBUG_MEMORY_INIT
> bool "Debug memory initialisation" if EXPERT
> default !EXPERT



--
~Randy

2012-08-13 21:31:36

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Mon, Aug 13, 2012 at 01:57:11PM -0700, Thai Bui wrote:
> Putting DEBUG_INITCALL config option to compile out the command-line option
> "initcall_debug".

Can you explain the benfits of this please?
>
> Signed-off-by: Thai Bui <[email protected]>
> Reviewed-by: Josh Triplett <[email protected]>
> ---
> Documentation/kernel-parameters.txt | 3 ++-
> include/linux/init.h | 4 ++++
> init/main.c | 2 ++
> lib/Kconfig.debug | 9 +++++++++
> 4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index d99fd9c..3dbaf15 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> initcall_debug [KNL] Trace initcalls as they are executed. Useful
> for working out where the kernel is dying during
> - startup.
> + startup. DEBUG_INITCALL needs to be enabled in order
> + for this option to work.
>
> initrd= [BOOT] Specify the location of the initial ramdisk
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 6b95109..d2f31f1 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -157,7 +157,11 @@ void prepare_namespace(void);
>
> extern void (*late_time_init)(void);
>
> +#ifdef CONFIG_DEBUG_INITCALL
> extern bool initcall_debug;
> +#else
> +static const bool initcall_debug = false;
> +#endif /* CONFIG_DEBUG_INITCALL */
>
> #endif
>
> diff --git a/init/main.c b/init/main.c
> index ff49a6d..65837f7 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -648,8 +648,10 @@ static void __init do_ctors(void)
> #endif
> }
>
> +#ifdef CONFIG_DEBUG_INITCALL
> bool initcall_debug;
> core_param(initcall_debug, initcall_debug, bool, 0644);
> +#endif /* CONFIG_DEBUG_INITCALL */
>
> static char msgbuf[64];
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8745ac7..424ac93 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT
>
> If unsure, say N.
>
> +config DEBUG_INITCALL
> + bool "Debug initcalls as they are executed"
> + depends on DEBUG_KERNEL
> + help
> + Enable this for tracing initcalls during startup. Useful for working
> + out where the kernel is dying during startup.
> +
> + If unsure, say N
> +
> config DEBUG_MEMORY_INIT
> bool "Debug memory initialisation" if EXPERT
> default !EXPERT
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2012-08-13 22:12:25

by Thai Bui

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

Hi all,

I am as part of a capstone group at Portland State University is
working to tinify the kernel as small as possible. The ultimate goal
is to make the kernel small enough to use on micro-controller (or
under < 200k). This patch is one of them, it saves 176 bytes on a
minimal configuration of the kernel (the bzImage file was reduced from
363264 bytes to 363264 bytes applying this patch).

Aside from the purpose of reducing the size of the kernel. We are also
trying to clean up the kernel by making Kconfig options to compile out
the stuffs that aren't used often.

On Mon, Aug 13, 2012 at 5:21 PM, Konrad Rzeszutek Wilk
<[email protected]> wrote:
> On Mon, Aug 13, 2012 at 01:57:11PM -0700, Thai Bui wrote:
>> Putting DEBUG_INITCALL config option to compile out the command-line option
>> "initcall_debug".
>
> Can you explain the benfits of this please?
>>
>> Signed-off-by: Thai Bui <[email protected]>
>> Reviewed-by: Josh Triplett <[email protected]>
>> ---
>> Documentation/kernel-parameters.txt | 3 ++-
>> include/linux/init.h | 4 ++++
>> init/main.c | 2 ++
>> lib/Kconfig.debug | 9 +++++++++
>> 4 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index d99fd9c..3dbaf15 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>
>> initcall_debug [KNL] Trace initcalls as they are executed. Useful
>> for working out where the kernel is dying during
>> - startup.
>> + startup. DEBUG_INITCALL needs to be enabled in order
>> + for this option to work.
>>
>> initrd= [BOOT] Specify the location of the initial ramdisk
>>
>> diff --git a/include/linux/init.h b/include/linux/init.h
>> index 6b95109..d2f31f1 100644
>> --- a/include/linux/init.h
>> +++ b/include/linux/init.h
>> @@ -157,7 +157,11 @@ void prepare_namespace(void);
>>
>> extern void (*late_time_init)(void);
>>
>> +#ifdef CONFIG_DEBUG_INITCALL
>> extern bool initcall_debug;
>> +#else
>> +static const bool initcall_debug = false;
>> +#endif /* CONFIG_DEBUG_INITCALL */
>>
>> #endif
>>
>> diff --git a/init/main.c b/init/main.c
>> index ff49a6d..65837f7 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -648,8 +648,10 @@ static void __init do_ctors(void)
>> #endif
>> }
>>
>> +#ifdef CONFIG_DEBUG_INITCALL
>> bool initcall_debug;
>> core_param(initcall_debug, initcall_debug, bool, 0644);
>> +#endif /* CONFIG_DEBUG_INITCALL */
>>
>> static char msgbuf[64];
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 8745ac7..424ac93 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT
>>
>> If unsure, say N.
>>
>> +config DEBUG_INITCALL
>> + bool "Debug initcalls as they are executed"
>> + depends on DEBUG_KERNEL
>> + help
>> + Enable this for tracing initcalls during startup. Useful for working
>> + out where the kernel is dying during startup.
>> +
>> + If unsure, say N
>> +
>> config DEBUG_MEMORY_INIT
>> bool "Debug memory initialisation" if EXPERT
>> default !EXPERT
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/



--
Thai

2012-08-13 22:37:15

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On 08/13/2012 03:08 PM, Thai Bui wrote:
> Hi all,
>
> I am as part of a capstone group at Portland State University is working
> to tinify the kernel as small as possible. The ultimate goal is to make
> the kernel small enough to use on micro-controller (or under < 200k).
> This patch is one of them, it saves 176 bytes on a minimal configuration
> of the kernel (the bzImage file was reduced from 363264 bytes to 363264
> bytes applying this patch).
>
> Aside from the purpose of reducing the size of the kernel. We are also
> trying to clean up the kernel by making Kconfig options to compile out
> the stuffs that aren't used often.

IMO the kernel already has too many kconfig options.

Also, personally I would not merge a patch that saves so little memory,
especially on what I consider a very useful option.


> On Mon, Aug 13, 2012 at 5:21 PM, Konrad Rzeszutek Wilk
> <[email protected] <mailto:[email protected]>> wrote:
>
> On Mon, Aug 13, 2012 at 01:57:11PM -0700, Thai Bui wrote:
> > Putting DEBUG_INITCALL config option to compile out the
> command-line option
> > "initcall_debug".
>
> Can you explain the benfits of this please?
> >
> > Signed-off-by: Thai Bui <[email protected]
> <mailto:[email protected]>>
> > Reviewed-by: Josh Triplett <[email protected]
> <mailto:[email protected]>>
> > ---
> > Documentation/kernel-parameters.txt | 3 ++-
> > include/linux/init.h | 4 ++++
> > init/main.c | 2 ++
> > lib/Kconfig.debug | 9 +++++++++
> > 4 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> > index d99fd9c..3dbaf15 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes
> can also be entirely omitted.
> >
> > initcall_debug [KNL] Trace initcalls as they are executed.
> Useful
> > for working out where the kernel is dying
> during
> > - startup.
> > + startup. DEBUG_INITCALL needs to be enabled
> in order
> > + for this option to work.
> >
> > initrd= [BOOT] Specify the location of the initial
> ramdisk
> >
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 6b95109..d2f31f1 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -157,7 +157,11 @@ void prepare_namespace(void);
> >
> > extern void (*late_time_init)(void);
> >
> > +#ifdef CONFIG_DEBUG_INITCALL
> > extern bool initcall_debug;
> > +#else
> > +static const bool initcall_debug = false;
> > +#endif /* CONFIG_DEBUG_INITCALL */
> >
> > #endif
> >
> > diff --git a/init/main.c b/init/main.c
> > index ff49a6d..65837f7 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -648,8 +648,10 @@ static void __init do_ctors(void)
> > #endif
> > }
> >
> > +#ifdef CONFIG_DEBUG_INITCALL
> > bool initcall_debug;
> > core_param(initcall_debug, initcall_debug, bool, 0644);
> > +#endif /* CONFIG_DEBUG_INITCALL */
> >
> > static char msgbuf[64];
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 8745ac7..424ac93 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT
> >
> > If unsure, say N.
> >
> > +config DEBUG_INITCALL
> > + bool "Debug initcalls as they are executed"
> > + depends on DEBUG_KERNEL
> > + help
> > + Enable this for tracing initcalls during startup. Useful
> for working
> > + out where the kernel is dying during startup.
> > +
> > + If unsure, say N
> > +
> > config DEBUG_MEMORY_INIT
> > bool "Debug memory initialisation" if EXPERT
> > default !EXPERT
> > --

2012-08-14 01:18:35

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
> On 08/13/2012 03:08 PM, Thai Bui wrote:
> >Hi all,
> >
> >I am as part of a capstone group at Portland State University is working
> >to tinify the kernel as small as possible. The ultimate goal is to make
> >the kernel small enough to use on micro-controller (or under < 200k).
> >This patch is one of them, it saves 176 bytes on a minimal configuration
> >of the kernel (the bzImage file was reduced from 363264 bytes to 363264
> >bytes applying this patch).
> >
> >Aside from the purpose of reducing the size of the kernel. We are also
> >trying to clean up the kernel by making Kconfig options to compile out
> >the stuffs that aren't used often.
>
> IMO the kernel already has too many kconfig options.
>
> Also, personally I would not merge a patch that saves so little memory,
> especially on what I consider a very useful option.

I think Thai undersold his patch significantly; the *compressed* size
went down by 176 bytes, and the uncompressed size went down more than
that. And that's the savings starting from a very minimal kernel, not
starting from a defconfig kernel.

In any case, do you object to the introduction of a Kconfig option at
all, or to that option defaulting to off? In particular, would you
object if the option only showed up if EMBEDDED, and defaulted to y? At
that point, you could reasonably expect that most users and distros will
have it enabled, so you'll be able to count on asking people to enable
it and send you the output. Would that suffice?

The patch itself seems incredibly straightforward and non-invasive to
me; it just stubs out the global variable and lets GCC fold away all the
code.

At this point, the kernel is running out of major things to cut out to
save space; getting from ~200k (the current smallest kernel possible) to
much less than that will require a pile of patches that save anywhere
from a few hundred bytes to a few kilobytes. I certainly agree that
those patches need to avoid introducing too much complexity. However, I
don't think it makes sense to object to a patch that saves space solely
on the grounds that it doesn't save *more* space. That would make it
impossible to cut out small things, and small things add up.

- Josh Triplett

2012-08-14 12:03:44

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Mon, Aug 13, 2012 at 9:18 PM, Josh Triplett <[email protected]> wrote:
> On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
>> On 08/13/2012 03:08 PM, Thai Bui wrote:
>> >Hi all,
>> >
>> >I am as part of a capstone group at Portland State University is working
>> >to tinify the kernel as small as possible. The ultimate goal is to make
>> >the kernel small enough to use on micro-controller (or under < 200k).
>> >This patch is one of them, it saves 176 bytes on a minimal configuration
>> >of the kernel (the bzImage file was reduced from 363264 bytes to 363264
>> >bytes applying this patch).
>> >
>> >Aside from the purpose of reducing the size of the kernel. We are also
>> >trying to clean up the kernel by making Kconfig options to compile out
>> >the stuffs that aren't used often.
>>
>> IMO the kernel already has too many kconfig options.
>>
>> Also, personally I would not merge a patch that saves so little memory,
>> especially on what I consider a very useful option.
>
> I think Thai undersold his patch significantly; the *compressed* size
> went down by 176 bytes, and the uncompressed size went down more than
> that. And that's the savings starting from a very minimal kernel, not
> starting from a defconfig kernel.
>
> In any case, do you object to the introduction of a Kconfig option at
> all, or to that option defaulting to off? In particular, would you
> object if the option only showed up if EMBEDDED, and defaulted to y? At
> that point, you could reasonably expect that most users and distros will
> have it enabled, so you'll be able to count on asking people to enable
> it and send you the output. Would that suffice?

Hiding it behind EMBEDDED might be a start. From a distro perspective,
we actually use this particular option quite often so keeping the
ability to use it as you describe is important.

> The patch itself seems incredibly straightforward and non-invasive to
> me; it just stubs out the global variable and lets GCC fold away all the
> code.
>
> At this point, the kernel is running out of major things to cut out to
> save space; getting from ~200k (the current smallest kernel possible) to
> much less than that will require a pile of patches that save anywhere
> from a few hundred bytes to a few kilobytes. I certainly agree that
> those patches need to avoid introducing too much complexity. However, I
> don't think it makes sense to object to a patch that saves space solely
> on the grounds that it doesn't save *more* space. That would make it
> impossible to cut out small things, and small things add up.

If you're really going to pursue that, I'd suggest hiding the removals
behind a new option that most people won't set.

josh

2012-08-14 12:50:12

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Tue, Aug 14, 2012 at 08:03:41AM -0400, Josh Boyer wrote:
> On Mon, Aug 13, 2012 at 9:18 PM, Josh Triplett <[email protected]> wrote:
> > On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
> >> On 08/13/2012 03:08 PM, Thai Bui wrote:
> >> >Hi all,
> >> >
> >> >I am as part of a capstone group at Portland State University is working
> >> >to tinify the kernel as small as possible. The ultimate goal is to make
> >> >the kernel small enough to use on micro-controller (or under < 200k).
> >> >This patch is one of them, it saves 176 bytes on a minimal configuration
> >> >of the kernel (the bzImage file was reduced from 363264 bytes to 363264
> >> >bytes applying this patch).
> >> >
> >> >Aside from the purpose of reducing the size of the kernel. We are also
> >> >trying to clean up the kernel by making Kconfig options to compile out
> >> >the stuffs that aren't used often.
> >>
> >> IMO the kernel already has too many kconfig options.
> >>
> >> Also, personally I would not merge a patch that saves so little memory,
> >> especially on what I consider a very useful option.
> >
> > I think Thai undersold his patch significantly; the *compressed* size
> > went down by 176 bytes, and the uncompressed size went down more than
> > that. And that's the savings starting from a very minimal kernel, not
> > starting from a defconfig kernel.
> >
> > In any case, do you object to the introduction of a Kconfig option at
> > all, or to that option defaulting to off? In particular, would you
> > object if the option only showed up if EMBEDDED, and defaulted to y? At
> > that point, you could reasonably expect that most users and distros will
> > have it enabled, so you'll be able to count on asking people to enable
> > it and send you the output. Would that suffice?
>
> Hiding it behind EMBEDDED might be a start. From a distro perspective,
> we actually use this particular option quite often so keeping the
> ability to use it as you describe is important.

Fair enough.

> > The patch itself seems incredibly straightforward and non-invasive to
> > me; it just stubs out the global variable and lets GCC fold away all the
> > code.
> >
> > At this point, the kernel is running out of major things to cut out to
> > save space; getting from ~200k (the current smallest kernel possible) to
> > much less than that will require a pile of patches that save anywhere
> > from a few hundred bytes to a few kilobytes. I certainly agree that
> > those patches need to avoid introducing too much complexity. However, I
> > don't think it makes sense to object to a patch that saves space solely
> > on the grounds that it doesn't save *more* space. That would make it
> > impossible to cut out small things, and small things add up.
>
> If you're really going to pursue that, I'd suggest hiding the removals
> behind a new option that most people won't set.

Most of the other options added via this project have used EXPERT or
EMBEDDED. This one originally seemed enough like a debugging option to
warrant just using DEBUG_KERNEL and let it default to N, but it sounds
like this one needs to use EMBEDDED as well.

- Josh Triplett

2012-08-14 20:14:30

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On 08/13/2012 06:18 PM, Josh Triplett wrote:

> On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
>> On 08/13/2012 03:08 PM, Thai Bui wrote:
>>> Hi all,
>>>
>>> I am as part of a capstone group at Portland State University is working
>>> to tinify the kernel as small as possible. The ultimate goal is to make
>>> the kernel small enough to use on micro-controller (or under < 200k).
>>> This patch is one of them, it saves 176 bytes on a minimal configuration
>>> of the kernel (the bzImage file was reduced from 363264 bytes to 363264
>>> bytes applying this patch).
>>>
>>> Aside from the purpose of reducing the size of the kernel. We are also
>>> trying to clean up the kernel by making Kconfig options to compile out
>>> the stuffs that aren't used often.
>>
>> IMO the kernel already has too many kconfig options.
>>
>> Also, personally I would not merge a patch that saves so little memory,
>> especially on what I consider a very useful option.
>
> I think Thai undersold his patch significantly; the *compressed* size
> went down by 176 bytes, and the uncompressed size went down more than


Thanks. Good point.

> that. And that's the savings starting from a very minimal kernel, not
> starting from a defconfig kernel.
>
> In any case, do you object to the introduction of a Kconfig option at
> all, or to that option defaulting to off? In particular, would you
> object if the option only showed up if EMBEDDED, and defaulted to y? At
> that point, you could reasonably expect that most users and distros will
> have it enabled, so you'll be able to count on asking people to enable
> it and send you the output. Would that suffice?

It's not one patch that I object to. It's a "pile" of them.
and when does it stop? or does it go on ad infinitum?

One could make options to make many lines of code configurable,
but that would hardly be the right thing to do IMHO.

> The patch itself seems incredibly straightforward and non-invasive to
> me; it just stubs out the global variable and lets GCC fold away all the
> code.
>
> At this point, the kernel is running out of major things to cut out to
> save space; getting from ~200k (the current smallest kernel possible) to
> much less than that will require a pile of patches that save anywhere


a pile being how many patches (roughly)?

> from a few hundred bytes to a few kilobytes. I certainly agree that
> those patches need to avoid introducing too much complexity. However, I
> don't think it makes sense to object to a patch that saves space solely
> on the grounds that it doesn't save *more* space. That would make it
> impossible to cut out small things, and small things add up.


Yeah, I think that former Sen. Everett Dirksen said something like that. ;)


--
~Randy

2012-08-14 20:56:16

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Tue, Aug 14, 2012 at 01:13:00PM -0700, Randy Dunlap wrote:
> On 08/13/2012 06:18 PM, Josh Triplett wrote:
> > On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
> > In any case, do you object to the introduction of a Kconfig option at
> > all, or to that option defaulting to off? In particular, would you
> > object if the option only showed up if EMBEDDED, and defaulted to y? At
> > that point, you could reasonably expect that most users and distros will
> > have it enabled, so you'll be able to count on asking people to enable
> > it and send you the output. Would that suffice?
>
> It's not one patch that I object to. It's a "pile" of them.
> and when does it stop? or does it go on ad infinitum?

Sounds like you're describing Linux development in general, and I think
the same argument of "as long as people keep wanting to work on it"
applies.

> One could make options to make many lines of code configurable,
> but that would hardly be the right thing to do IMHO.

That seems like an argument better made about specific patches, rather
than as a blanket statement ignoring the details of any particular
patch. It seems reasonable to me to evaluate the tradeoff of complexity
versus space savings for each patch. A complex patch that saves very
little space certainly doesn't seem reasonable, and a simple patch that
saves a pile of space seems very reasonable. In this case, the space
savings seems reasonable enough to justify a patch that seems incredibly
non-invasive. If the patch had a diffstat in the hundreds of lines, I'd
understand the complaint.

> > The patch itself seems incredibly straightforward and non-invasive to
> > me; it just stubs out the global variable and lets GCC fold away all the
> > code.
> >
> > At this point, the kernel is running out of major things to cut out to
> > save space; getting from ~200k (the current smallest kernel possible) to
> > much less than that will require a pile of patches that save anywhere
>
> a pile being how many patches (roughly)?

At the moment, the team has a half-dozen patches in flight. How many
more will happen in the future depends on how well the remaining parts
of a minimal kernel partition into large, self-contained, removable
chunks.

In any case, could we perhaps pull this conversation back down out of
the abstract and go back to discussing the specific patch in question?

- Josh Triplett

2012-08-14 21:26:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On 08/14/2012 01:55 PM, Josh Triplett wrote:

> On Tue, Aug 14, 2012 at 01:13:00PM -0700, Randy Dunlap wrote:
>> On 08/13/2012 06:18 PM, Josh Triplett wrote:
>>> On Mon, Aug 13, 2012 at 03:39:54PM -0700, Randy Dunlap wrote:
>>> In any case, do you object to the introduction of a Kconfig option at
>>> all, or to that option defaulting to off? In particular, would you
>>> object if the option only showed up if EMBEDDED, and defaulted to y? At
>>> that point, you could reasonably expect that most users and distros will
>>> have it enabled, so you'll be able to count on asking people to enable
>>> it and send you the output. Would that suffice?
>>
>> It's not one patch that I object to. It's a "pile" of them.
>> and when does it stop? or does it go on ad infinitum?
>
> Sounds like you're describing Linux development in general, and I think
> the same argument of "as long as people keep wanting to work on it"
> applies.

touche.

>> One could make options to make many lines of code configurable,
>> but that would hardly be the right thing to do IMHO.
>
> That seems like an argument better made about specific patches, rather
> than as a blanket statement ignoring the details of any particular
> patch. It seems reasonable to me to evaluate the tradeoff of complexity
> versus space savings for each patch. A complex patch that saves very
> little space certainly doesn't seem reasonable, and a simple patch that
> saves a pile of space seems very reasonable. In this case, the space
> savings seems reasonable enough to justify a patch that seems incredibly
> non-invasive. If the patch had a diffstat in the hundreds of lines, I'd
> understand the complaint.
>
>>> The patch itself seems incredibly straightforward and non-invasive to
>>> me; it just stubs out the global variable and lets GCC fold away all the
>>> code.
>>>
>>> At this point, the kernel is running out of major things to cut out to
>>> save space; getting from ~200k (the current smallest kernel possible) to
>>> much less than that will require a pile of patches that save anywhere
>>
>> a pile being how many patches (roughly)?
>
> At the moment, the team has a half-dozen patches in flight. How many
> more will happen in the future depends on how well the remaining parts
> of a minimal kernel partition into large, self-contained, removable
> chunks.
>
> In any case, could we perhaps pull this conversation back down out of
> the abstract and go back to discussing the specific patch in question?


Surely. I have no gross objection to this specific patch.

regards,
--
~Randy

2012-08-14 22:14:37

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On Tue, Aug 14, 2012 at 02:25:07PM -0700, Randy Dunlap wrote:
> Surely. I have no gross objection to this specific patch.

OK. Thanks!

PATCHv2 will hide the option behind EMBEDDED and make it default to y.

- Josh Triplett

2012-08-15 18:02:35

by Tim Bird

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

Thai,

How can I find out more about this project to reduce the size of the Linux
kernel? I have been doing some research at Sony on ways to reduce
the Linux kernel to very small sizes, without introducing persistent source
code changes or config options. The reason for this is to avoid the inevitable
resistance to mainlining hundreds or thousands of config options and associated
code changes.

In a nutshell what I do is select individual data fields or functions
to eliminate or constrain,
then let automated tools impose that constraint throughout a working copy
of the kernel source tree. (That is, my tools are run for each build
of the kernel,
so the source changes they make are not persistent). The compiler
then optimizes
out the fields, and code associated with them. This allows me to impose a
constraint (similar to a config option), without interjecting it into
the Kconfig
system or the mainline kernel source tree.

The code is in very early proof-of-concept condition right now.
I am currently experimenting to see how robust this system is, in terms of
both code correctness for a specific kernel version, and applicability
of technique
over a range of kernel versions.

I'd like to describe my work to your group in more detail, and see if there's
any overlap between what I'm doing and what you're doing. My own goal
is kernel runtime footprint, including static and dynamic memory usage of 500k.

Is there any web site describing your work, or staging area for your patches?
Would you be willing to share your .config files?

Thanks for any information you can provide about your work.
-- Tim

On Mon, Aug 13, 2012 at 3:11 PM, Thai Bui <[email protected]> wrote:
> Hi all,
>
> I am as part of a capstone group at Portland State University is
> working to tinify the kernel as small as possible. The ultimate goal
> is to make the kernel small enough to use on micro-controller (or
> under < 200k). This patch is one of them, it saves 176 bytes on a
> minimal configuration of the kernel (the bzImage file was reduced from
> 363264 bytes to 363264 bytes applying this patch).
>
> Aside from the purpose of reducing the size of the kernel. We are also
> trying to clean up the kernel by making Kconfig options to compile out
> the stuffs that aren't used often.
>
> On Mon, Aug 13, 2012 at 5:21 PM, Konrad Rzeszutek Wilk
> <[email protected]> wrote:
>> On Mon, Aug 13, 2012 at 01:57:11PM -0700, Thai Bui wrote:
>>> Putting DEBUG_INITCALL config option to compile out the command-line option
>>> "initcall_debug".
>>
>> Can you explain the benfits of this please?
>>>
>>> Signed-off-by: Thai Bui <[email protected]>
>>> Reviewed-by: Josh Triplett <[email protected]>
>>> ---
>>> Documentation/kernel-parameters.txt | 3 ++-
>>> include/linux/init.h | 4 ++++
>>> init/main.c | 2 ++
>>> lib/Kconfig.debug | 9 +++++++++
>>> 4 files changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index d99fd9c..3dbaf15 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -1026,7 +1026,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>>
>>> initcall_debug [KNL] Trace initcalls as they are executed. Useful
>>> for working out where the kernel is dying during
>>> - startup.
>>> + startup. DEBUG_INITCALL needs to be enabled in order
>>> + for this option to work.
>>>
>>> initrd= [BOOT] Specify the location of the initial ramdisk
>>>
>>> diff --git a/include/linux/init.h b/include/linux/init.h
>>> index 6b95109..d2f31f1 100644
>>> --- a/include/linux/init.h
>>> +++ b/include/linux/init.h
>>> @@ -157,7 +157,11 @@ void prepare_namespace(void);
>>>
>>> extern void (*late_time_init)(void);
>>>
>>> +#ifdef CONFIG_DEBUG_INITCALL
>>> extern bool initcall_debug;
>>> +#else
>>> +static const bool initcall_debug = false;
>>> +#endif /* CONFIG_DEBUG_INITCALL */
>>>
>>> #endif
>>>
>>> diff --git a/init/main.c b/init/main.c
>>> index ff49a6d..65837f7 100644
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -648,8 +648,10 @@ static void __init do_ctors(void)
>>> #endif
>>> }
>>>
>>> +#ifdef CONFIG_DEBUG_INITCALL
>>> bool initcall_debug;
>>> core_param(initcall_debug, initcall_debug, bool, 0644);
>>> +#endif /* CONFIG_DEBUG_INITCALL */
>>>
>>> static char msgbuf[64];
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 8745ac7..424ac93 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -769,6 +769,15 @@ config DEBUG_WRITECOUNT
>>>
>>> If unsure, say N.
>>>
>>> +config DEBUG_INITCALL
>>> + bool "Debug initcalls as they are executed"
>>> + depends on DEBUG_KERNEL
>>> + help
>>> + Enable this for tracing initcalls during startup. Useful for working
>>> + out where the kernel is dying during startup.
>>> +
>>> + If unsure, say N
>>> +
>>> config DEBUG_MEMORY_INIT
>>> bool "Debug memory initialisation" if EXPERT
>>> default !EXPERT
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
> --
> Thai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2012-08-22 19:02:47

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] boot: Put initcall_debug into its own Kconfig option DEBUG_INITCALL

On 08/14/2012 03:14 PM, Josh Triplett wrote:

> On Tue, Aug 14, 2012 at 02:25:07PM -0700, Randy Dunlap wrote:
>> Surely. I have no gross objection to this specific patch.
>
> OK. Thanks!
>
> PATCHv2 will hide the option behind EMBEDDED and make it default to y.
>
> - Josh Triplett


BTW, I think that it would be Good if you would Cc: the
author(s) of initcall_debug (Ingo, Arjan ?).

thanks,
--
~Randy