2007-09-14 10:35:53

by Denys Vlasenko

[permalink] [raw]
Subject: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

Hi Tapio,

You are the author of these files. Are you still maintaining them?
If not, do you know who is the current maintainer?

These two object files hold the biggest data objects in the whole Linux kernel
after lockdep:

text data bss dec hex filename
1258 160516 0 161774 277ee ./drivers/usb/misc/emi26.o
1504 209296 0 210800 33770 ./drivers/usb/misc/emi62.o

Basically, these are big arrays of the following structures:

typedef struct _INTEL_HEX_RECORD
{
__u32 length;
__u32 address;
__u32 type;
__u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
} INTEL_HEX_RECORD;

I suggest the following optimizations:

Change structure to

typedef struct _INTEL_HEX_RECORD
{
__u8 type;
__u8 length;
__u16 address;
__u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
} INTEL_HEX_RECORD __attribute__((__packed__));

Store gzip compressed tables and unpack them at load time.

Declare them const and __initdata.

I can do it if there is no active maintainer.
--
vda


2007-09-18 12:22:01

by Clemens Ladisch

[permalink] [raw]
Subject: Re: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

Denys Vlasenko wrote:
> Hi Tapio,
>
> You are the author of these files. Are you still maintaining them?

His newer email address that I found with Google is dead, too.

> These two object files hold the biggest data objects in the whole
> Linux kernel
>
> Basically, these are big arrays of the following structures:
>
> typedef struct _INTEL_HEX_RECORD
> {
> __u32 length;
> __u32 address;
> __u32 type;
> __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> } INTEL_HEX_RECORD;
>
> I suggest the following optimizations:
>
> Change structure to
>
> typedef struct _INTEL_HEX_RECORD
> {
> __u8 type;
> __u8 length;
> __u16 address;
> __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> } INTEL_HEX_RECORD __attribute__((__packed__));
>
> Store gzip compressed tables and unpack them at load time.
>
> Declare them const and __initdata.

I have a patch somewhere that moves the firmware code to userspace and changes
the drivers to use request_firmware().


Regards,
Clemens

2007-09-19 02:48:16

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

On Fri, 14 Sep 2007 11:35:34 BST, Denys Vlasenko said:
> Hi Tapio,
>
> You are the author of these files. Are you still maintaining them?
> If not, do you know who is the current maintainer?

> These two object files hold the biggest data objects in the whole Linux kernel
> after lockdep:
>
> text data bss dec hex filename
> 1258 160516 0 161774 277ee ./drivers/usb/misc/emi26.o
> 1504 209296 0 210800 33770 ./drivers/usb/misc/emi62.o
>
> Basically, these are big arrays of the following structures:
>
> typedef struct _INTEL_HEX_RECORD
> {
> __u32 length;
> __u32 address;
> __u32 type;
> __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> } INTEL_HEX_RECORD;
>
> I suggest the following optimizations:
>
> Change structure to

I suggest moving those out of the kernel entirely and use the firmware loader
support to bring it in from userspace like all the *other* firmware blobs.

'INTEL_HEX_RECORD' just *screams* 'microcode' ;)


Attachments:
(No filename) (226.00 B)

2007-09-28 04:38:46

by Greg KH

[permalink] [raw]
Subject: Re: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

On Fri, Sep 14, 2007 at 11:35:34AM +0100, Denys Vlasenko wrote:
> Hi Tapio,
>
> You are the author of these files. Are you still maintaining them?
> If not, do you know who is the current maintainer?
>
> These two object files hold the biggest data objects in the whole Linux kernel
> after lockdep:
>
> text data bss dec hex filename
> 1258 160516 0 161774 277ee ./drivers/usb/misc/emi26.o
> 1504 209296 0 210800 33770 ./drivers/usb/misc/emi62.o
>
> Basically, these are big arrays of the following structures:
>
> typedef struct _INTEL_HEX_RECORD
> {
> __u32 length;
> __u32 address;
> __u32 type;
> __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> } INTEL_HEX_RECORD;
>
> I suggest the following optimizations:
>
> Change structure to
>
> typedef struct _INTEL_HEX_RECORD
> {
> __u8 type;
> __u8 length;
> __u16 address;
> __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> } INTEL_HEX_RECORD __attribute__((__packed__));

Only if you redo the whole firmware image too :)

What is this really hurting? It's only relevant if you load the
specific module, if you have this device type. It's a firmware blob,
nothing really interesting at all.

thanks,

greg k-h

2007-09-28 11:34:48

by Denys Vlasenko

[permalink] [raw]
Subject: Re: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

On Friday 28 September 2007 05:41, Greg KH wrote:
> On Fri, Sep 14, 2007 at 11:35:34AM +0100, Denys Vlasenko wrote:
> > Hi Tapio,
> >
> > You are the author of these files. Are you still maintaining them?
> > If not, do you know who is the current maintainer?
> >
> > These two object files hold the biggest data objects in the whole Linux kernel
> > after lockdep:
> >
> > text data bss dec hex filename
> > 1258 160516 0 161774 277ee ./drivers/usb/misc/emi26.o
> > 1504 209296 0 210800 33770 ./drivers/usb/misc/emi62.o
> >
> > Basically, these are big arrays of the following structures:
> >
> > typedef struct _INTEL_HEX_RECORD
> > {
> > __u32 length;
> > __u32 address;
> > __u32 type;
> > __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> > } INTEL_HEX_RECORD;
> >
> > I suggest the following optimizations:
> >
> > Change structure to
> >
> > typedef struct _INTEL_HEX_RECORD
> > {
> > __u8 type;
> > __u8 length;
> > __u16 address;
> > __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> > } INTEL_HEX_RECORD __attribute__((__packed__));
>
> Only if you redo the whole firmware image too :)

I did. It wasn't hard.

> What is this really hurting? It's only relevant if you load the
> specific module

By this logic, no space wastage in modules is worth fixing.
--
vda

2007-09-28 19:44:08

by Greg KH

[permalink] [raw]
Subject: Re: drivers/usb/misc/emi*.c have the biggest data objects in the whole tree

On Fri, Sep 28, 2007 at 12:34:29PM +0100, Denys Vlasenko wrote:
> On Friday 28 September 2007 05:41, Greg KH wrote:
> > On Fri, Sep 14, 2007 at 11:35:34AM +0100, Denys Vlasenko wrote:
> > > Hi Tapio,
> > >
> > > You are the author of these files. Are you still maintaining them?
> > > If not, do you know who is the current maintainer?
> > >
> > > These two object files hold the biggest data objects in the whole Linux kernel
> > > after lockdep:
> > >
> > > text data bss dec hex filename
> > > 1258 160516 0 161774 277ee ./drivers/usb/misc/emi26.o
> > > 1504 209296 0 210800 33770 ./drivers/usb/misc/emi62.o
> > >
> > > Basically, these are big arrays of the following structures:
> > >
> > > typedef struct _INTEL_HEX_RECORD
> > > {
> > > __u32 length;
> > > __u32 address;
> > > __u32 type;
> > > __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> > > } INTEL_HEX_RECORD;
> > >
> > > I suggest the following optimizations:
> > >
> > > Change structure to
> > >
> > > typedef struct _INTEL_HEX_RECORD
> > > {
> > > __u8 type;
> > > __u8 length;
> > > __u16 address;
> > > __u8 data[MAX_INTEL_HEX_RECORD_LENGTH];
> > > } INTEL_HEX_RECORD __attribute__((__packed__));
> >
> > Only if you redo the whole firmware image too :)
>
> I did. It wasn't hard.

Great, send me a patch then :)

> > What is this really hurting? It's only relevant if you load the
> > specific module
>
> By this logic, no space wastage in modules is worth fixing.

No, that's not true. Be realistic here please.

thanks,

greg k-h