Discussion:
[PATCH 0/2] do not select KALLSYMS_ALL
(too old to reply)
Artem Bityutskiy
2011-03-30 08:40:14 UTC
Permalink
UBI and UBIFS have an option to compile-in debugging support. When we enable
debugging, compile-in various assertions, self-check functions and test modes.
We also compile in additional verbose error messages, flash dumps and stack
dumps.

When we have debugging support enabled, we want to have nice stackdumps, so
we selected KALLSYMS_ALL. But this causes various problems like unmet direct
dependencies, reported by Randy:

warning: (UBIFS_FS_DEBUG && LOCKDEP && LATENCYTOP) selects KALLSYMS_ALL which
has unmet direct dependencies (DEBUG_KERNEL && KALLSYMS)

I quickly looked at KALLSYMS_ALL and it is not immediately clear what exactly
is the difference between KALLSYMS and KALLSYMS_ALL. And there is this
CONFIG_KALLSYMS_EXTRA_PASS symbol, which is declared "temporary workaround"
in the Kconfig help. This shows that this stuff is messy.

I personally think KALLSYMS_ALL should be just merged with KALLSYMS and
disappear - we should have only one option. CONFIG_KALLSYMS_EXTRA_PASS should
die as well.

But I don't want to delve into that, I choose to just forget about KALLSYMS_ALL
and select KALLSYMS in UBI/UBIFS which seems to be enough for us.

Artem.
Artem Bityutskiy
2011-03-30 08:40:16 UTC
Permalink
From: Artem Bityutskiy <***@nokia.com>

All UBI needs is to make sure we stacktraces when UBI debugging
is enabled. It is enough to select KALLSYMS for this, KALLSYMS_ALL
is not necessary.

And the current Kconfig line we have:

select KALLSYMS_ALL if KALLSYMS && DEBUG_KERNEL

is just too complex to be sane and right. But this "if" part there
is needed to prevent "unmet direct dependency" warnings, because
KALLSYMS_ALL depends on KALLSYMS and DEBUG_KERNEL, so we cannot
just select KALLSYMS_ALL.

Anyway, this feels messy, and we do not seem to really need KALLSYMS_ALL,
so select KALLSYMS instead.

Signed-off-by: Artem Bityutskiy <***@nokia.com>
---
drivers/mtd/ubi/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 6abeb4f..4dcc752 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -56,7 +56,7 @@ config MTD_UBI_DEBUG
bool "UBI debugging"
depends on SYSFS
select DEBUG_FS
- select KALLSYMS_ALL if KALLSYMS && DEBUG_KERNEL
+ select KALLSYMS
help
This option enables UBI debugging.
--
1.7.2.3
Randy Dunlap
2011-03-30 22:41:33 UTC
Permalink
Post by Artem Bityutskiy
All UBI needs is to make sure we stacktraces when UBI debugging
is enabled. It is enough to select KALLSYMS for this, KALLSYMS_ALL
is not necessary.
select KALLSYMS_ALL if KALLSYMS && DEBUG_KERNEL
is just too complex to be sane and right. But this "if" part there
is needed to prevent "unmet direct dependency" warnings, because
KALLSYMS_ALL depends on KALLSYMS and DEBUG_KERNEL, so we cannot
just select KALLSYMS_ALL.
Anyway, this feels messy, and we do not seem to really need KALLSYMS_ALL,
so select KALLSYMS instead.
---
drivers/mtd/ubi/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 6abeb4f..4dcc752 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -56,7 +56,7 @@ config MTD_UBI_DEBUG
bool "UBI debugging"
depends on SYSFS
select DEBUG_FS
- select KALLSYMS_ALL if KALLSYMS && DEBUG_KERNEL
+ select KALLSYMS
help
This option enables UBI debugging.
--
Acked-by: Randy Dunlap <***@oracle.com>

Thanks.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Artem Bityutskiy
2011-03-30 08:40:15 UTC
Permalink
From: Artem Bityutskiy <***@nokia.com>

All UBIFS needs is to make sure we stacktraces when UBIFS debugging
is enabled. It is enough to select KALLSYMS for this, KALLSYMS_ALL
is not necessary. Moreover, Randy Dunlap reported that UBIFS causes
the following Kconfig dependency warning:

warning: (UBIFS_FS_DEBUG && LOCKDEP && LATENCYTOP) selects KALLSYMS_ALL
which has unmet direct dependencies (DEBUG_KERNEL && KALLSYMS)

The reason is that KALLSYMS_ALL requires DEBUG_KERNEL and KALLSYMS, so
ideally, to select KALLSYMS_ALL we'd need to select DEBUG_KERNEL and
KALLSYMS first.

This seems to be too much to select. The easiest way to go is to forget
about KALLSYMS_ALL and just select KALLSYMS when UBIFS debugging is
enabled - that should be enough for stackdumps.

Reported-by: Randy Dunlap <***@oracle.com>
Signed-off-by: Artem Bityutskiy <***@nokia.com>
---
fs/ubifs/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index d744090..f8b0160 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -47,7 +47,7 @@ config UBIFS_FS_DEBUG
bool "Enable debugging support"
depends on UBIFS_FS
select DEBUG_FS
- select KALLSYMS_ALL
+ select KALLSYMS
help
This option enables UBIFS debugging support. It makes sure various
assertions, self-checks, debugging messages and test modes are compiled
--
1.7.2.3
Randy Dunlap
2011-03-30 22:41:30 UTC
Permalink
Post by Artem Bityutskiy
All UBIFS needs is to make sure we stacktraces when UBIFS debugging
is enabled. It is enough to select KALLSYMS for this, KALLSYMS_ALL
is not necessary. Moreover, Randy Dunlap reported that UBIFS causes
warning: (UBIFS_FS_DEBUG && LOCKDEP && LATENCYTOP) selects KALLSYMS_ALL
which has unmet direct dependencies (DEBUG_KERNEL && KALLSYMS)
The reason is that KALLSYMS_ALL requires DEBUG_KERNEL and KALLSYMS, so
ideally, to select KALLSYMS_ALL we'd need to select DEBUG_KERNEL and
KALLSYMS first.
This seems to be too much to select. The easiest way to go is to forget
about KALLSYMS_ALL and just select KALLSYMS when UBIFS debugging is
enabled - that should be enough for stackdumps.
---
fs/ubifs/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index d744090..f8b0160 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -47,7 +47,7 @@ config UBIFS_FS_DEBUG
bool "Enable debugging support"
depends on UBIFS_FS
select DEBUG_FS
- select KALLSYMS_ALL
+ select KALLSYMS
help
This option enables UBIFS debugging support. It makes sure various
assertions, self-checks, debugging messages and test modes are compiled
--
Acked-by: Randy Dunlap <***@oracle.com>

Thanks.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Artem Bityutskiy
2011-03-31 06:29:53 UTC
Permalink
Post by Artem Bityutskiy
diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index d744090..f8b0160 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -47,7 +47,7 @@ config UBIFS_FS_DEBUG
bool "Enable debugging support"
depends on UBIFS_FS
select DEBUG_FS
- select KALLSYMS_ALL
+ select KALLSYMS
help
This option enables UBIFS debugging support. It makes sure vari=
ous
Post by Artem Bityutskiy
assertions, self-checks, debugging messages and test modes are =
compiled
Post by Artem Bityutskiy
--=20
=20
Pushed to UBI / UBIFS trees. These will show-up in linux-next a bit
later (around -rc2, if it will be usable enough), and then I'll send
this to Linus.

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E=
=D1=86=D0=BA=D0=B8=D0=B9)
Paulo Marques
2011-03-31 14:18:58 UTC
Permalink
Post by Artem Bityutskiy
[...]
I personally think KALLSYMS_ALL should be just merged with KALLSYMS and
disappear - we should have only one option. CONFIG_KALLSYMS_EXTRA_PASS should
die as well.
That sounds a little too extreme...

KALLSYMS is useful for most kernels, since it provides nice readable
stack dumps for panics and BUG's.

KALLSYMS_ALL adds a lot of extra symbols that can be useful mostly to
development kernels and shouldn't be used to add unnecessary bloat to
user kernels.

Now as for CONFIG_KALLSYMS_EXTRA_PASS: to build the kallsyms table, the
build process first links a kernel image with an empty kallsyms table
and use that to fetch information for all the symbols.

It then uses that information to build the table with the right size,
and links it again. If everything goes ok, this new version as all the
symbols in the correct places and the final table can be built with the
correct addresses.

The final linking should produce the same result as only the data on the
kallsyms table changed, but not its size.

However, there have been bugs in the past with section alignments and
symbol reordering for symbols with the same address, etc., etc. that
make this final table not have the exact same size, and the build fails
with an inconsistent kallsyms data message. At this point, the user can
turn on the CONFIG_KALLSYMS_EXTRA_PASS and temporarily solve the problem
while the developers find the correct fix. Without this option, in this
situation the kernel would simply fail the compilation.

All this has been stable for a while and this option hasn't been needed
recently (AFAIK), but if there is some bug in some new binutils or
something, the option might be needed again.
--
Paulo Marques - www.grupopie.com

"Who is general Failure and why is he reading my disk?"
Artem Bityutskiy
2011-04-01 14:40:33 UTC
Permalink
Post by Paulo Marques
Post by Artem Bityutskiy
[...]
I personally think KALLSYMS_ALL should be just merged with KALLSYMS and
disappear - we should have only one option. CONFIG_KALLSYMS_EXTRA_PASS should
die as well.
That sounds a little too extreme...
KALLSYMS is useful for most kernels, since it provides nice readable
stack dumps for panics and BUG's.
KALLSYMS_ALL adds a lot of extra symbols that can be useful mostly to
development kernels and shouldn't be used to add unnecessary bloat to
user kernels.
OK, thanks.
Post by Paulo Marques
Now as for CONFIG_KALLSYMS_EXTRA_PASS: to build the kallsyms table, the
build process first links a kernel image with an empty kallsyms table
and use that to fetch information for all the symbols.
It then uses that information to build the table with the right size,
and links it again. If everything goes ok, this new version as all the
symbols in the correct places and the final table can be built with the
correct addresses.
The final linking should produce the same result as only the data on the
kallsyms table changed, but not its size.
However, there have been bugs in the past with section alignments and
symbol reordering for symbols with the same address, etc., etc. that
make this final table not have the exact same size, and the build fails
with an inconsistent kallsyms data message. At this point, the user can
turn on the CONFIG_KALLSYMS_EXTRA_PASS and temporarily solve the problem
while the developers find the correct fix. Without this option, in this
situation the kernel would simply fail the compilation.
All this has been stable for a while and this option hasn't been needed
recently (AFAIK), but if there is some bug in some new binutils or
something, the option might be needed again.
Thanks for explanation!

But... why on earth this option is in Kconfig then, if this is only
about extra pass during the kernel _compilation_ ? This and the vague
help message in Kconfig help section are very misleading. This should
not be in Kconfig at all then, it should be purely a Makefile thing!
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
Artem Bityutskiy
2011-04-01 14:56:16 UTC
Permalink
Post by Paulo Marques
[...]
I personally think KALLSYMS_ALL should be just merged with KALLSYMS=
and
Post by Paulo Marques
disappear - we should have only one option. CONFIG_KALLSYMS_EXTRA_P=
ASS should
Post by Paulo Marques
die as well.
=20
That sounds a little too extreme...
=20
KALLSYMS is useful for most kernels, since it provides nice readable
stack dumps for panics and BUG's.
=20
KALLSYMS_ALL adds a lot of extra symbols that can be useful mostly to
development kernels and shouldn't be used to add unnecessary bloat to
user kernels.
Well, ok, I've measured how much is this "a lot". On an embedded arm
platform this makes the kernel only 1.5% larger.

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E=
=D1=86=D0=BA=D0=B8=D0=B9)
Artem Bityutskiy
2011-04-01 15:01:44 UTC
Permalink
Post by Artem Bityutskiy
Well, ok, I've measured how much is this "a lot". On an embedded arm
platform this makes the kernel only 1.5% larger.=20
But in absolute numbers this is 70KiB in my case, which is indeed
considerable amount. So, I agree that it makes sense to keep this as a
separate option.

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E=
=D1=86=D0=BA=D0=B8=D0=B9)
Ricard Wanderlof
2011-04-01 15:11:36 UTC
Permalink
Post by Artem Bityutskiy
Post by Artem Bityutskiy
Well, ok, I've measured how much is this "a lot". On an embedded arm
platform this makes the kernel only 1.5% larger.
But in absolute numbers this is 70KiB in my case, which is indeed
considerable amount. So, I agree that it makes sense to keep this as a
separate option.
Yes there are platforms where space is tight and 70 KiB could make or
break a given setup.

/Ricard
--
Ricard Wolf Wanderlöf ricardw(at)axis.com
Axis Communications AB, Lund, Sweden www.axis.com
Phone +46 46 272 2016 Fax +46 46 13 61 30
Paulo Marques
2011-04-01 15:34:42 UTC
Permalink
Post by Artem Bityutskiy
Post by Artem Bityutskiy
Well, ok, I've measured how much is this "a lot". On an embedded arm
platform this makes the kernel only 1.5% larger.
But in absolute numbers this is 70KiB in my case, which is indeed
considerable amount. So, I agree that it makes sense to keep this as a
separate option.
Yes, and this depends a lot on the kernel configuration options you've
selected (and that 1.5% of the kernel size, might be 50%~100% of the
kallsyms table).

IIRC, I had configurations where the KALLSYMS_ALL option was increasing
the kallsyms table from something like 150kB to above 500kB (before
compression). This was a long time ago though and I can't say exactly
what was the configuration that made this happen.

As for the CONFIG_KALLSYMS_EXTRA_PASS, I think the original idea (it was
not mine) was that it should be off by default and then the user would
need to turn it on when something went wrong.

With an automatic makefile mechanism, the problem would go unnoticed and
it just wouldn't be fixed, increasing the kernel compile time for
everyone who hits the same troublesome configuration.
--
Paulo Marques - www.grupopie.com

"As far as we know, our computer has never had an undetected error."
Weisert
Artem Bityutskiy
2011-04-01 15:42:29 UTC
Permalink
Post by Artem Bityutskiy
Well, ok, I've measured how much is this "a lot". On an embedded a=
rm
Post by Artem Bityutskiy
platform this makes the kernel only 1.5% larger.=20
=20
But in absolute numbers this is 70KiB in my case, which is indeed
considerable amount. So, I agree that it makes sense to keep this a=
s a
Post by Artem Bityutskiy
separate option.
=20
Yes, and this depends a lot on the kernel configuration options you'v=
e
selected (and that 1.5% of the kernel size, might be 50%~100% of the
kallsyms table).
=20
IIRC, I had configurations where the KALLSYMS_ALL option was increasi=
ng
the kallsyms table from something like 150kB to above 500kB (before
compression). This was a long time ago though and I can't say exactly
what was the configuration that made this happen.
Yes, thanks, I agree that having 2 separate options is OK.
As for the CONFIG_KALLSYMS_EXTRA_PASS, I think the original idea (it =
was
not mine) was that it should be off by default and then the user woul=
d
need to turn it on when something went wrong.
Yes, but my point is that this feature is about the build and it does
not affect run-time, so it should not be in Kconfig.
With an automatic makefile mechanism, the problem would go unnoticed =
and
it just wouldn't be fixed, increasing the kernel compile time for
everyone who hits the same troublesome configuration.
Yes, the build system may print a message:

Your symbols table is screwed, this is a bug, report about it. As a
temporary workaround use "make KALLSYMS_EXTRA_PASS=3D1"

Or something like that.

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E=
=D1=86=D0=BA=D0=B8=D0=B9)
Paulo Marques
2011-04-01 15:54:53 UTC
Permalink
Post by Artem Bityutskiy
Post by Paulo Marques
[...]
With an automatic makefile mechanism, the problem would go unnoticed and
it just wouldn't be fixed, increasing the kernel compile time for
everyone who hits the same troublesome configuration.
Your symbols table is screwed, this is a bug, report about it. As a
temporary workaround use "make KALLSYMS_EXTRA_PASS=1"
Or something like that.
Yes, I think that would work too, and it would even prevent the option
from being on for no good reason.
--
Paulo Marques - www.grupopie.com

"Nostalgia isn't what it used to be."
Artem Bityutskiy
2011-04-04 07:12:09 UTC
Permalink
Post by Artem Bityutskiy
[...]
With an automatic makefile mechanism, the problem would go unnotic=
ed and
Post by Artem Bityutskiy
it just wouldn't be fixed, increasing the kernel compile time for
everyone who hits the same troublesome configuration.
=20
=20
Your symbols table is screwed, this is a bug, report about it. As a
temporary workaround use "make KALLSYMS_EXTRA_PASS=3D1"
=20
Or something like that.
=20
Yes, I think that would work too, and it would even prevent the optio=
n
from being on for no good reason.
I'll try to find some time and cook a patch.

--=20
Best Regards,
Artem Bityutskiy (=D0=90=D1=80=D1=82=D1=91=D0=BC =D0=91=D0=B8=D1=82=D1=8E=
=D1=86=D0=BA=D0=B8=D0=B9)

Continue reading on narkive:
Loading...