Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Matt Mackall]

We've added 12k of new initdata to allnoconfig builds for i8042 DMI
lookup tables:

http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers

It looks like each table entry is > 320 bytes as we reserve four 80-byte
slots in each for DMI match strings. 

-- 
Mathematics is the supreme nostalgia of our time.

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Jiri Kosina]

On Fri, 24 Oct 2008, Matt Mackall wrote:

> We've added 12k of new initdata to allnoconfig builds for i8042 DMI
> lookup tables:
> 
> http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers
> 
> It looks like each table entry is > 320 bytes as we reserve four 80-byte
> slots in each for DMI match strings. 

Umm, and what is the actual problem with that, really?

OK, we can remove it from .init, but then it will be rotting in memory 
forever, which is quite sub-optimal, when this kind of DMI information is 
needed only during initialization.

We can make ->ident strings shorter, they do not serve any real purpose, 
and are there just for readability. Is it worth that?

We can make a config option which will guard existence of these lists, 
such as CONFIG_INPUT_BLACKLISTS, let id default to y and depend on 
EMBEDDED.

Any other reasonable ideas?

-- 
Jiri Kosina
SUSE Labs

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Matt Mackall]

On Sat, 2008-10-25 at 00:15 +0200, Jiri Kosina wrote:
> On Fri, 24 Oct 2008, Matt Mackall wrote:
> 
> > We've added 12k of new initdata to allnoconfig builds for i8042 DMI
> > lookup tables:
> > 
> > http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers
> > 
> > It looks like each table entry is > 320 bytes as we reserve four 80-byte
> > slots in each for DMI match strings. 
> 
> Umm, and what is the actual problem with that, really?

The actual problem is that if the kernel grows by 12k every time a
developer says "what's the big deal?" the kernel will become very large
indeed.

> OK, we can remove it from .init, but then it will be rotting in memory 
> forever, which is quite sub-optimal, when this kind of DMI information is 
> needed only during initialization.

I wasn't complaining that they were in init, but rather that they were
12k. For something like 37 entries. The entry size is ridiculous and
looks to have grown by a factor of 10 since 2.6.27. What, as they say,
is up with that?

It looks like David Woodhouse is to blame:

commit d945b697d0eea5a811ec299c5f1a25889bb0242b
Automatic MODULE_ALIAS() for DMI match tables.

This is probably also responsible for most of the growth in x86 I
mentioned elsewhere, so it's about 25-30k damage in total. David?

-- 
Mathematics is the supreme nostalgia of our time.

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Dmitry Torokhov]

On Friday 24 October 2008, Matt Mackall wrote:
> On Sat, 2008-10-25 at 00:15 +0200, Jiri Kosina wrote:
> > On Fri, 24 Oct 2008, Matt Mackall wrote:
> > 
> > > We've added 12k of new initdata to allnoconfig builds for i8042 DMI
> > > lookup tables:
> > > 
> > > http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers
> > > 
> > > It looks like each table entry is > 320 bytes as we reserve four 80-byte
> > > slots in each for DMI match strings. 
> > 
> > Umm, and what is the actual problem with that, really?
> 
> The actual problem is that if the kernel grows by 12k every time a
> developer says "what's the big deal?" the kernel will become very large
> indeed.
> 
> > OK, we can remove it from .init, but then it will be rotting in memory 
> > forever, which is quite sub-optimal, when this kind of DMI information is 
> > needed only during initialization.
> 
> I wasn't complaining that they were in init, but rather that they were
> 12k. For something like 37 entries. The entry size is ridiculous and
> looks to have grown by a factor of 10 since 2.6.27. What, as they say,
> is up with that?
> 
> It looks like David Woodhouse is to blame:
> 
> commit d945b697d0eea5a811ec299c5f1a25889bb0242b
> Automatic MODULE_ALIAS() for DMI match tables.
> 
> This is probably also responsible for most of the growth in x86 I
> mentioned elsewhere, so it's about 25-30k damage in total. David?
> 

I think that the net effect of this change is positive. While it is
true that the size of the kernel image grew we do discard more memory
(most of DMI tables are marked __initdata) than we were able to do
before DMI strings were embedded into dmi_strmatch so the footprint of
the running kernel now should be smaller. Having said this I wonder if
we could reduce the length of these strings form 79 to let's say 39. 

I would like to get rid of .ident strings in cases when drivers don't
use them for anything, this should free some more memory.

-- 
Dmitry

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Matt Mackall]

On Fri, 2008-10-24 at 20:46 -0400, Dmitry Torokhov wrote:
> On Friday 24 October 2008, Matt Mackall wrote:
> > On Sat, 2008-10-25 at 00:15 +0200, Jiri Kosina wrote:
> > > On Fri, 24 Oct 2008, Matt Mackall wrote:
> > > 
> > > > We've added 12k of new initdata to allnoconfig builds for i8042 DMI
> > > > lookup tables:
> > > > 
> > > > http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers
> > > > 
> > > > It looks like each table entry is > 320 bytes as we reserve four 80-byte
> > > > slots in each for DMI match strings. 
> > > 
> > > Umm, and what is the actual problem with that, really?
> > 
> > The actual problem is that if the kernel grows by 12k every time a
> > developer says "what's the big deal?" the kernel will become very large
> > indeed.
> > 
> > > OK, we can remove it from .init, but then it will be rotting in memory 
> > > forever, which is quite sub-optimal, when this kind of DMI information is 
> > > needed only during initialization.
> > 
> > I wasn't complaining that they were in init, but rather that they were
> > 12k. For something like 37 entries. The entry size is ridiculous and
> > looks to have grown by a factor of 10 since 2.6.27. What, as they say,
> > is up with that?
> > 
> > It looks like David Woodhouse is to blame:
> > 
> > commit d945b697d0eea5a811ec299c5f1a25889bb0242b
> > Automatic MODULE_ALIAS() for DMI match tables.
> > 
> > This is probably also responsible for most of the growth in x86 I
> > mentioned elsewhere, so it's about 25-30k damage in total. David?
> > 
> 
> I think that the net effect of this change is positive.

Sure. But it's still suddenly one of the biggest data structures in the
kernel image. And I expect we'll keep accumulating these sorts of
tables.

>  While it is
> true that the size of the kernel image grew we do discard more memory
> (most of DMI tables are marked __initdata) than we were able to do
> before DMI strings were embedded into dmi_strmatch so the footprint of
> the running kernel now should be smaller. Having said this I wonder if
> we could reduce the length of these strings form 79 to let's say 39. 

The sad thing is that we've gone from using len(x) + 4 bytes to 79
bytes, where the average x appears to have been... 4 (counting the fact
that most match slots are empty!). If we used strstr rather than strcmp,
we could probably get by with < 8 bytes per id (and maybe roll some
entries together).

Also, painfully, we end the lists with 320+ byte empty entries.

Ideally, we'd find a way to (a) store variable-length string constants
in init sections and (b) teach modpost about following pointers.

Something like this would work inside code but sadly not inside
declarations:

#define ISTR(s) ({static char c[] __attribute__ ((section("__initdata__"))) = s;c;})

> I would like to get rid of .ident strings in cases when drivers don't
> use them for anything, this should free some more memory.

Interestingly, those still live in the kernel image after init as
they're pointers to string constants. And most of them are purely
redundant.

-- 
Mathematics is the supreme nostalgia of our time.

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[David Woodhouse]

On Fri, 2008-10-24 at 22:31 -0500, Matt Mackall wrote:
> Ideally, we'd find a way to (a) store variable-length string constants
> in init sections and (b) teach modpost about following pointers.

Doing (b) shouldn't be _so_ hard. Modpost already trawls through the
relocations, looking for naughty ones. Making it actually _do_ them when
it needs to shouldn't be impossible.

> Something like this would work inside code but sadly not inside
> declarations:
> 
> #define ISTR(s) ({static char c[] __attribute__ ((section("__initdata__"))) = s;c;})

Yeah, I played with that for a while, but gave up.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[Jiri Kosina]

On Fri, 24 Oct 2008, Dmitry Torokhov wrote:

> I would like to get rid of .ident strings in cases when drivers don't
> use them for anything, this should free some more memory.

Yes, that could be nice thing to do indeed. They are usually used only to 
store a human-readable description of the device anyway, which is then not 
printed anywhere, and therefore a plain C comment should be sufficient 
instead.

It will require fixing dmi_check_system() though, as it's one of those 
rare functions actually using ->ident field for detecting end of the list.

-- 
Jiri Kosina
SUSE Labs

Re: Bloatwatch 2.6.28-rc1: i8042 DMI lookup tables

[David Woodhouse]

On Fri, 2008-10-24 at 18:13 -0500, Matt Mackall wrote:
> On Sat, 2008-10-25 at 00:15 +0200, Jiri Kosina wrote:
> > On Fri, 24 Oct 2008, Matt Mackall wrote:
> > 
> > > We've added 12k of new initdata to allnoconfig builds for i8042 DMI
> > > lookup tables:
> > > 
> > > http://www.selenic.com/bloatwatch/?cmd=compare;v1=2.6.27;v2=2.6.28-rc1;part=/built-in/drivers
> > > 
> > > It looks like each table entry is > 320 bytes as we reserve four 80-byte
> > > slots in each for DMI match strings. 
> > 
> > Umm, and what is the actual problem with that, really?
> 
> The actual problem is that if the kernel grows by 12k every time a
> developer says "what's the big deal?" the kernel will become very large
> indeed.
> 
> > OK, we can remove it from .init, but then it will be rotting in memory 
> > forever, which is quite sub-optimal, when this kind of DMI information is 
> > needed only during initialization.
> 
> I wasn't complaining that they were in init, but rather that they were
> 12k. For something like 37 entries. The entry size is ridiculous and
> looks to have grown by a factor of 10 since 2.6.27. What, as they say,
> is up with that?
> 
> It looks like David Woodhouse is to blame:
> 
> commit d945b697d0eea5a811ec299c5f1a25889bb0242b
> Automatic MODULE_ALIAS() for DMI match tables.
> 
> This is probably also responsible for most of the growth in x86 I
> mentioned elsewhere, so it's about 25-30k damage in total. David?

There's an assessment of this in the commit comment which you omitted.

It compresses well, and it's __initdata. So it takes up very little
space in the bzImage, and is dropped fairly quickly during boot. So the
amount of time it's _actually_ taking up any noticeable amount of memory
is really quite small.

But if someone wants to extend modpost so that it can handle relocations
and we can use string _pointers_ in the dmi tables, that would be good.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation