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.
-Jelte On Thu, 2005-02-24 at 09:42, Greg Walton wrote: > On Wed, 23 Feb 2005 13:46:17 -0500 (EST), D. Hugh Redelmeier > <[EMAIL PROTECTED]> wrote: > > | From: David Gesswein <[EMAIL PROTECTED]> > > | Subject: Re: [ivtv-devel] #0.2.0-rc3g and #0.3.2e fixes > > | > > | > The main change is to make as many seq arrays as possible static const. > > | > If they had a non-const part, I had to leave them on the stack. David > > | > went to the trouble of kmallocing everything. > > | > > > | I had missed one of the big arrays so started moving smaller stuff. > > | The non const ones were about 300 bytes so may be ok with the other > > | stuff fixed. Does anybody know the proper way to figure out how much > > | statck was used? > > > > No, I don't know the proper or conventional way (if any). > > > > But here is an approach that seems to work. I just made this up now, > > so there is probably a better way. Remember, my system is an X86_64 > > running Fedora Core 3 with a 2.6 kernel. > > > > 1) compile the modules to assembler (.s) files > > > > 2) look at the assembly code to find the size of stack being > > allocated. > > > > To compile to assembly code, I first needed to know how the code is > > supposed to be compiled. The output of "make" is very terse -- it no > snip! > > http://lkml.org/lkml/2004/5/14/34 > > I tried the guts of that script against the cx driver and got numbers > like Hugh got. > I'm outta my league on this but I thought it might be helpful. > > greg > > > > longer shows you the actual commands executed. > > > > 1a: clean up any residue from previous builds > > make clean > > > > 1b: do a make with verbose output to see how the files are compiled. > > I need to capture the log. You could do this with script (I used > > a different tool): > > script > > make KBUILD_VERBOSE=1 > > exit > > > > 1c: look at the script output ("typescript") to find the directory the > > compile was done in: > > make[1]: Entering directory `/lib/modules/2.6.10-1.741_FC3/build' > > and the actual gcc command: > > gcc > > -Wp,-MD,/home/hugh/tba/ivtv/ivtv-0.3.2e/driver/.cx25840-driver.o.d > > -nostdinc -iwithprefix include -D__KERNEL__ -Iinclude -Wall > > -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Os > > -fomit-frame-pointer -g -mno-red-zone -mcmodel=kernel -pipe > > -fno-reorder-blocks -Wno-sign-compare -funit-at-a-time > > -Wdeclaration-after-statement -DMODULE -DKBUILD_BASENAME=cx25840_driver > > -DKBUILD_MODNAME=cx25840 -c -o > > /home/hugh/tba/ivtv/ivtv-0.3.2e/driver/.tmp_cx25840-driver.o > > /home/hugh/tba/ivtv/ivtv-0.3.2e/driver/cx25840-driver.c > > > > 1d: issue a variant of that command to create assembly code (.s) file. > > You need to set where the output goes -- change the -o flag's > > operand. > > You need to add the -S flag, replacing the -c flag, to tell it > > that you want assembly code. > > > > bash > > cd /lib/modules/2.6.10-1.741_FC3/build > > gcc > > -Wp,-MD,/home/hugh/tba/ivtv/ivtv-0.3.2e/driver/.cx25840-driver.o.d > > -nostdinc -iwithprefix include -D__KERNEL__ -Iinclude -Wall > > -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Os > > -fomit-frame-pointer -g -mno-red-zone -mcmodel=kernel -pipe > > -fno-reorder-blocks -Wno-sign-compare -funit-at-a-time > > -Wdeclaration-after-statement -DMODULE -DKBUILD_BASENAME=cx25840_driver > > -DKBUILD_MODNAME=cx25840 -S -o ~/cx25840-driver.s > > /home/hugh/tba/ivtv/ivtv-0.3.2e/driver/cx25840-driver.c > > exit > > > > The assembly code should now be in wherever your -o operand > > pointed. > > > > Now you need to peer at the assembly code for function prologues that > > allocate stack space. On my system, it looks as if space is allocated > > by instructions like: > > subq $2632, %rsp > > This says: subtract a constant from the stack pointer register (stacks > > grow down). Look for big constants. I can imagine that some > > functions don't have a subq, but only if they are allocating a very > > small amount of stack. (Or if they use run-time sized arrays, a > > recent feature of C; I assume that the driver does not use this.) > > > > 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 > > > > [Remember that I'm on x86_64 so pointers take twice as much memory as > > on i386. To accommodate this, x86_64 linux allows twice as much stack > > space as i386.] > > > > David: > > > > My intuition is that kmalloc is to be avoided if possible. The call > > is much more expensive than stack allocation. And it can fail. And > > it can cause fragmentation. And you have to be careful to ensure that > > the memory is freed exactly once. So it is best to limit it to > > allocating big things. For some meaning of "big". > > > > If there are two or more big things in a function, perhaps one kmalloc > > can be used to allocate space for all at once. > > > > I would guess that the top 3 of these functions should be fixed to use > > less stack. Probably using kmalloc. The fourth might be acceptable > > the way it is. This depends on the call graph, something that I am > > ignoring. > > > > Since all your changes were to cx25840-driver.c, perhaps I have given > > you all the information you were asking for. > > > > I think that it would be good for you to resubmit your patch, perhaps > > improved. It does address an important problem. > > > > Thanks. > > > > ------------------------------------------------------- > > 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 > > > > > ------------------------------------------------------- > 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 ------------------------------------------------------- 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
