linux-kernel janitorial RFP: Mark static arrays as const
Here is a possible opportunity to reduce data usage in the kernel. $ git grep -P -n '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]' drivers/ | \ grep -v __initdata | \ wc -l 3250 Meaning there are ~3000 declarations of arrays with what appears to be file static const content that are not marked const. So there are many static arrays that could be marked const to move the compiled object code from data to text minimizing the total amount of exposed r/w data. However, I do not know of a mechanism using coccinelle to determine whether or not any of these static declarations are ever modified. So it appears that each instance of these declarations might need manual inspection. But for arrays declared inside functions, it's much more likely that the static declaration without const is done with the intent to modify the array: (note the difference in the git grep with a leading '^\s+') $ git grep -Pn '^\s+static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]' drivers/ | \ grep -v __initdata | \ wc -l 323 ------------- For instance: (head -10 of the git grep for file statics) drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32, 16, 8, 4, 2, 1 }; drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = { drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[] = { drivers/accessibility/speakup/speakup_acntpc.c:35:static unsigned int synth_portlist[] = { 0x2a8, 0 }; drivers/accessibility/speakup/speakup_decpc.c:133:static int synth_portlist[] = { 0x340, 0x350, 0x240, 0x250, 0 }; drivers/accessibility/speakup/speakup_dectlk.c:110:static int ap_defaults[] = {122, 89, 155, 110, 208, 240, 200, 106, 306}; drivers/accessibility/speakup/speakup_dectlk.c:111:static int g5_defaults[] = {86, 81, 86, 84, 81, 80, 83, 83, 73}; drivers/accessibility/speakup/speakup_dtlk.c:34:static unsigned int synth_portlist[] = { drivers/accessibility/speakup/speakup_keypc.c:34:static unsigned int synth_portlist[] = { 0x2a8, 0 }; drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = { For drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32, 16, 8, 4, 2, 1 }; masks is only used in static function say_key and should be const and perhaps the declaration might be better moved into that function. For drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = { funcvals is only used in static function spk_handle_help and should be const and perhaps the declaration might be better moved into that function. For drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[] = { spkup_handler is only used in static function do_spkup and should be const and perhaps the declaration might be better moved into that function. etc... for speakup For drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = { array ac_props is assigned as a reference in acpi_ac_add as a "const enum power_supply_property *" member of a struct power_supply_desc. ------------- For instance: (head -10 of the git grep for function statics) drivers/acpi/apei/apei-base.c:781: static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c"; drivers/block/amiflop.c:1051: static unsigned char CRCTable1[] = { drivers/block/amiflop.c:1070: static unsigned char CRCTable2[] = { drivers/block/drbd/drbd_nl.c:872: static char units[] = { 'K', 'M', 'G', 'T', 'P', 'E' }; drivers/block/drbd/drbd_proc.c:224: static char write_ordering_chars[] = { drivers/block/drbd/drbd_receiver.c:4363: static enum drbd_conns c_tab[] = { drivers/char/pcmcia/synclink_cs.c:3717: static unsigned char patterns[] = drivers/cpufreq/intel_pstate.c:1515: static int silvermont_freq_table[] = { drivers/cpufreq/intel_pstate.c:1530: static int airmont_freq_table[] = { drivers/dma/xgene-dma.c:360: static u8 flyby_type[] = { Some of these could be const, but some could not. For instance: For drivers/acpi/apei/apei-base.c:781: static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c"; whea_uuid_str is assigned as a reference in "int apei_osc_setup(void)" a struct acpi_osc_context where .uuid_str is not declared as const char *.
On Tue, 2 Mar 2021, Joe Perches wrote:
Here is a possible opportunity to reduce data usage in the kernel.
Does it actually reduce data usage? julia
$ git grep -P -n '^static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]' drivers/ | \ grep -v __initdata | \ wc -l 3250
Meaning there are ~3000 declarations of arrays with what appears to be file static const content that are not marked const.
So there are many static arrays that could be marked const to move the compiled object code from data to text minimizing the total amount of exposed r/w data.
However, I do not know of a mechanism using coccinelle to determine whether or not any of these static declarations are ever modified.
So it appears that each instance of these declarations might need manual inspection.
But for arrays declared inside functions, it's much more likely that the static declaration without const is done with the intent to modify the array:
(note the difference in the git grep with a leading '^\s+')
$ git grep -Pn '^\s+static\s+(?!const|struct)(?:\w+\s+){1,3}\w+\s*\[\s*\]' drivers/ | \ grep -v __initdata | \ wc -l 323
------------- For instance: (head -10 of the git grep for file statics)
drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32, 16, 8, 4, 2, 1 }; drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = { drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[] = { drivers/accessibility/speakup/speakup_acntpc.c:35:static unsigned int synth_portlist[] = { 0x2a8, 0 }; drivers/accessibility/speakup/speakup_decpc.c:133:static int synth_portlist[] = { 0x340, 0x350, 0x240, 0x250, 0 }; drivers/accessibility/speakup/speakup_dectlk.c:110:static int ap_defaults[] = {122, 89, 155, 110, 208, 240, 200, 106, 306}; drivers/accessibility/speakup/speakup_dectlk.c:111:static int g5_defaults[] = {86, 81, 86, 84, 81, 80, 83, 83, 73}; drivers/accessibility/speakup/speakup_dtlk.c:34:static unsigned int synth_portlist[] = { drivers/accessibility/speakup/speakup_keypc.c:34:static unsigned int synth_portlist[] = { 0x2a8, 0 }; drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
For drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = { 32, 16, 8, 4, 2, 1 };
masks is only used in static function say_key and should be const and perhaps the declaration might be better moved into that function.
For drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = {
funcvals is only used in static function spk_handle_help and should be const and perhaps the declaration might be better moved into that function.
For drivers/accessibility/speakup/main.c:2059:static spkup_hand spkup_handler[] = {
spkup_handler is only used in static function do_spkup and should be const and perhaps the declaration might be better moved into that function.
etc... for speakup
For drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
array ac_props is assigned as a reference in acpi_ac_add as a "const enum power_supply_property *" member of a struct power_supply_desc.
------------- For instance: (head -10 of the git grep for function statics)
drivers/acpi/apei/apei-base.c:781: static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c"; drivers/block/amiflop.c:1051: static unsigned char CRCTable1[] = { drivers/block/amiflop.c:1070: static unsigned char CRCTable2[] = { drivers/block/drbd/drbd_nl.c:872: static char units[] = { 'K', 'M', 'G', 'T', 'P', 'E' }; drivers/block/drbd/drbd_proc.c:224: static char write_ordering_chars[] = { drivers/block/drbd/drbd_receiver.c:4363: static enum drbd_conns c_tab[] = { drivers/char/pcmcia/synclink_cs.c:3717: static unsigned char patterns[] = drivers/cpufreq/intel_pstate.c:1515: static int silvermont_freq_table[] = { drivers/cpufreq/intel_pstate.c:1530: static int airmont_freq_table[] = { drivers/dma/xgene-dma.c:360: static u8 flyby_type[] = {
Some of these could be const, but some could not. For instance:
For drivers/acpi/apei/apei-base.c:781: static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c";
whea_uuid_str is assigned as a reference in "int apei_osc_setup(void)" a struct acpi_osc_context where .uuid_str is not declared as const char *.
_______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
On Tue, 2021-03-02 at 22:41 +0100, Julia Lawall wrote:
On Tue, 2 Mar 2021, Joe Perches wrote:
Here is a possible opportunity to reduce data usage in the kernel.
Does it actually reduce data usage?
Yes, at least for gcc. For instance: $ gcc --version gcc (Ubuntu 10.2.0-13ubuntu1) 10.2.0 And with the diff below (x86-64 defconfig with hwmon/pmbus and max20730) $ diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c index 9dd3dd79bc18..b824eab95456 100644 --- a/drivers/hwmon/pmbus/max20730.c +++ b/drivers/hwmon/pmbus/max20730.c @@ -434,7 +434,7 @@ static long direct_to_val(u16 w, enum pmbus_sensor_classes > return d; } -static u32 max_current[][5] = { +static const u32 max_current[][5] = { [max20710] = { 6200, 8000, 9700, 11600 }, [max20730] = { 13000, 16600, 20100, 23600 }, [max20734] = { 21000, 27000, 32000, 38000 }, $ size drivers/hwmon/pmbus/max20730.o* text data bss dec hex filename 9245 256 0 9501 251d drivers/hwmon/pmbus/max20730.o.gcc.new 9149 352 0 9501 251d drivers/hwmon/pmbus/max20730.o.gcc.old with clang-11 it doesn't seem to make a difference: $ /usr/bin/clang --version Ubuntu clang version 11.0.0-2 $ size drivers/hwmon/pmbus/max20730.o* text data bss dec hex filename 9166 256 1 9423 24cf drivers/hwmon/pmbus/max20730.o.clang-11.new 9166 256 1 9423 24cf drivers/hwmon/pmbus/max20730.o.clang-11.old
participants (2)
-
Joe Perches -
Julia Lawall