Hi Takashi,
There is a lot of data structures in that code,
and most of them seems to be read-only.I added const modifiers to most of such places:
text data bss dec hex filename
106315 179564 36 285915 45cdb snd-hda-intel.o
283051 2624 36 285711 45c0f snd-hda-intel_patched.oPatch is attached.
It moves "static struct hda_codec_preset *hda_preset_tables[]"
from hda_patch.h to hda_codec.c, and then adds
#include "hda_patch.h"
in a few .c files so that definitions of e.g.
const struct hda_codec_preset snd_hda_preset_analog[]
are checked to match declarations in hda_patch.hThe rest of the patch (bulk of it) adds "const"
in many places.Patch is compile tested. Please apply.
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda
At Fri, 14 Sep 2007 18:48:05 +0100,
Sorry for the late reply.
First, thanks for your patch. Although I have also a similar patch
pending on my tree, but it wasn't applied, because I'd like to mark
these functions/data rather as __devinit*. And, sadly, init and const
don't like with each other. So, my plan is to apply __devinit but
without const.But, please don't hurry up to mark everything init yet now. I've been
working on the dynamic codec parser on user-space and this would
involve with some functions that are now apparently init functions.Since this isn't a critical issue, let's postpone it as a post-2.6.24
stuff.thanks,
Takashi
-
Unless we will go to the pains of implementing __devrodata,
Yes, I see. const as it stands is not very useful in kernel anyway
(only a small code reduction sometimes).
ro or rw, the data is still taking space.Well, maybe someday ld will be sooo clever that it will actually
merge rodata which is identical, but so far it is not implemented.
--
vda
-
This is kinda odd. Why did the _text_ size increase by constifying?
-
The data got converted from data to text because they were made const.
Making stuff const unfortunately does not make them disappear
albeit the embedded folks would have liked that.Sam
-
Which is odd. How can data become code? Or does 'text' actually
-
More precisely: size utility adds up all readonly code and
readonly data sections it sees and shows it as "text".ELF is not as rigid as old a.out (which had only one text, one data
and one bss segment per .o file IIRC), but size was born in a.out days,
so it sort of "translates" ELF into a.out.
--
vda
-
Try size -A instead.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
-
thanks, didn't know that.
$ size -A busybox
busybox :
section size addr
.init 28 134512788
.text 633051 134512816
.fini 23 135145867
.rodata 139422 135145904
.eh_frame 156 135285328
.ctors 8 135286784
.dtors 8 135286792
.jcr 4 135286800
.got.plt 12 135286804
.data 1019 135286816
.bss 10724 135287840
Total 784455addr in decimal?! wow...
--
vda
-
Try size -x instead.
-
After additional testing I found a place where
I forgot to add 'const', and build throws warnings
at me.Updated patch is attached.
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda
The SND_HDA_PRESETS define doesn't seem useful.
It's only used once.diff -urp linux-2.6.23-rc6/sound/pci/hda/hda_codec.c linux-2.6.23-rc6.bigdata/sound/pci/hda/hda_codec.c
--- linux-2.6.23-rc6/sound/pci/hda/hda_codec.c 2007-07-09 00:32:17.000000000 +0100
+++ linux-2.6.23-rc6.bigdata/sound/pci/hda/hda_codec.c 2007-09-14 18:32:24.000000000 +0100
@@ -57,6 +57,10 @@ static struct hda_vendor_id hda_vendor_i/* codec presets */
#include "hda_patch.h"
+static const struct hda_codec_preset *const hda_preset_tables[] = {
+ SND_HDA_PRESETS,
+ NULL
+};
diff -urp linux-2.6.23-rc6/sound/pci/hda/hda_patch.h linux-2.6.23-rc6.bigdata/sound/pci/hda/hda_patch.h
--- linux-2.6.23-rc6/sound/pci/hda/hda_patch.h 2007-07-09 00:32:17.000000000 +0100
+++ linux-2.6.23-rc6.bigdata/sound/pci/hda/hda_patch.h 2007-09-14 18:30:17.000000000 +0100
@@ -1,32 +1,30 @@
/*
- * HDA Patches - included by hda_codec.c
+ * HDA Patches
*//* Realtek codecs */
-extern struct hda_codec_preset snd_hda_preset_realtek[];
+extern const struct hda_codec_preset snd_hda_preset_realtek[];
/* C-Media codecs */
-extern struct hda_codec_preset snd_hda_preset_cmedia[];
+extern const struct hda_codec_preset snd_hda_preset_cmedia[];
/* Analog Devices codecs */
-extern struct hda_codec_preset snd_hda_preset_analog[];
+extern const struct hda_codec_preset snd_hda_preset_analog[];
/* SigmaTel codecs */
-extern struct hda_codec_preset snd_hda_preset_sigmatel[];
+extern const struct hda_codec_preset snd_hda_preset_sigmatel[];
/* SiLabs 3054/3055 modem codecs */
-extern struct hda_codec_preset snd_hda_preset_si3054[];
+extern const struct hda_codec_preset snd_hda_preset_si3054[];
/* ATI HDMI codecs */
-extern struct hda_codec_preset snd_hda_preset_atihdmi[];
+extern const struct hda_codec_preset snd_hda_preset_atihdmi[];
/* Conexant audio codec */
-extern struct hda_codec_preset snd_hda_preset_conexant[];
+extern const struct hda_codec_preset snd_hda_preset_conexant[];
/* VIA codecs */
-extern struct hda_codec_pre...
It is defined in .h file and used in .c file.
It is made so because defining static data variables in .h file
is a bad style in general and in this case will result in build-time
warnings in particular.Therefore definition of hda_preset_tables[] is moved to .c file.
It looks like this:
static const struct hda_codec_preset *const hda_preset_tables[] = {
snd_hda_preset_realtek,
snd_hda_preset_cmedia,
snd_hda_preset_analog,
snd_hda_preset_sigmatel,
snd_hda_preset_si3054,
snd_hda_preset_atihdmi,
snd_hda_preset_conexant,
snd_hda_preset_via,
NULL
};I want to make it easier for people to add new snd_hda_preset_XXX.
I don't want them to be forced to add it in hda_patch.h first,
and then go to hda_codec.c and add it there too.Therefore I turned this list into a #define which sits in hda_patch.h:
#define SND_HDA_PRESETS \
snd_hda_preset_realtek, \
snd_hda_preset_cmedia, \
snd_hda_preset_analog, \
snd_hda_preset_sigmatel, \
snd_hda_preset_si3054, \
snd_hda_preset_atihdmi, \
snd_hda_preset_conexant, \
snd_hda_preset_viaNow if you want to add yet another snd_hda_preset_XXX, you
don't need to touch hda_codec.c.Original code was achieving the same by cheating: it has
static const struct hda_codec_preset *hda_preset_tables[] = {...}
in hda_patch.h.
--
vda
-
