[I recommend that folks quote only relevant parts of previous
messages. That makes for shorter messages that are easier to read.]
| From: Jelte van der Hoek <[EMAIL PROTECTED]>
|
| I made a change to the cx25840 driver that changes the seq arrays to
| function calls (and uses an additional array to buffer the settings), I
| was still testing this but it seems to work. I don't have time to make a
| patch against the 0.3.2e and test it right now, but I'll do it tonight
| after I can test it.
Do you know if that change is worthwhile? How big are those arrays?
In some cases, you are moving work from compile time to runtime -- not
a great tradeoff.
How are you allocating the additional array? If statically, you need
to use locking of some kind to prevent interference. If kmalloc, that
takes care to avoid various dangers (as I explained in my earlier
message).
| > On Wed, 23 Feb 2005 13:46:17 -0500 (EST), D. Hugh Redelmeier
| > > Here are the top consumers of stack space in
| > > ivtv-0.3.2e/driver/cx25840-driver.c after my changes, but not Davids.
| > > I don't know the call graph, so I don't know if these need to be added
| > > together.
| > >
| > > 2632 setting_sequencer
| > > 1576 cx25840_detect_client
| > > 1384 cx25840_command
| > > 456 set_default_values
setting_sequencer is the biggest pig. Its main consumers of stack
space appear to be:
char buff[number_of_registers*5+1]; // 1816 bytes
__u8 reg_buff[number_of_registers+2]; // 365 bytes
__u8 dirt[number_of_registers]; // 363 bytes
I don't immediately see where cx25840_detect_client is taking all the
space. Hmmm. I guess it is due to inlining. So I repeat my stack
hog analysis with inlining turned off (-fno-inline)
Now I have a different hog list:
2632 setting_sequencer
1576 load_aud_fw
1376 cx25840_command
1312 do_cx25840_dump
456 set_default_values
load_aud_fw's main consumers of stack seem to be:
char fw_buffer[1024]={0x08, 0x02}; // 1024 bytes
char result_ascii[2*sizeof(result)+1]; // 129 bytes
char result[64]; // 64 bytes
cx25840_command is next. In this case, seq arrays seem to be the
culprit. But they may be affordable if the other hogs are dealt with.
do_cx25840_dump:
char buff[5*256+10], // 1290 bytes
set_default_values:
char buff[66*5+1]; // 331 bytes
u8 i2c_buffer[66]; // 66 bytes
I have identified the problem variables. I have not analyzed how they
can be replaced. Does anyone have insights, or should I just dig in?
BTW, as a stylistic issue, I find it disturbing that the code has
"magic numbers" in it -- unexplained large integers. What are 1024,
0x08, 0x02, 64, 256, 5, 10, 66? These ought to be documented and
given names.
In fact, there is a lot that I'd change in this file. Maybe you don't
want me to start because who knows where I'd stop. I am coming at
this based on no knowledge of the driver but fairly strong ideas about
C.
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
ivtv-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ivtv-devel
