* [STACK] >3k call path in ide
@ 2004-06-09 12:29 Jörn Engel
2004-06-15 23:34 ` [PATCH] " Randy.Dunlap
0 siblings, 1 reply; 14+ messages in thread
From: Jörn Engel @ 2004-06-09 12:29 UTC (permalink / raw)
To: B.Zolnierkiewicz, linux-ide; +Cc: linux-kernel
Bartlomiej, can you put ide_config on a diet?
stackframes for call path too long (3052):
size function
0 client_reg_t->event_handler
1168 ide_config
12 ide_register_hw
44 ide_unregister
12 ide_unregister_subdriver
0 pnpide_init
0 pnp_register_driver
0 driver_register
20 bus_add_driver
16 driver_attach
72 tty_register_device
0 class_simple_device_add
0 class_device_register
16 class_device_add
0 kobject_add
0 kobject_hotplug
132 call_usermodehelper
80 wait_for_completion
84 schedule
16 __put_task_struct
20 audit_free
36 audit_log_start
16 __kmalloc
0 __get_free_pages
28 __alloc_pages
284 try_to_free_pages
0 out_of_memory
0 mmput
16 exit_aio
0 __put_ioctx
16 do_munmap
0 split_vma
36 vma_adjust
0 fput
0 __fput
0 locks_remove_flock
12 panic
0 sys_sync
0 sync_inodes
308 sync_inodes_sb
0 do_writepages
128 mpage_writepages
4 write_boundary_block
0 ll_rw_block
28 submit_bh
0 bio_alloc
88 mempool_alloc
256 wakeup_bdflush
20 pdflush_operation
0 printk
0 preempt_schedule
84 schedule
Jörn
--
When you close your hand, you own nothing. When you open it up, you
own the whole world.
-- Li Mu Bai in Tiger & Dragon
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] [STACK] >3k call path in ide 2004-06-09 12:29 [STACK] >3k call path in ide Jörn Engel @ 2004-06-15 23:34 ` Randy.Dunlap 2004-06-16 7:11 ` Florian Schirmer 0 siblings, 1 reply; 14+ messages in thread From: Randy.Dunlap @ 2004-06-15 23:34 UTC (permalink / raw) To: Jörn Engel, akpm; +Cc: B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 9 Jun 2004 14:29:21 +0200 Jörn Engel wrote: | Bartlomiej, can you put ide_config on a diet? | | stackframes for call path too long (3052): | size function | 0 client_reg_t->event_handler | 1168 ide_config Here's a patch for ide_config(), the worst offender in this call chain. Reduce large stack usage in ide_config() by using kmalloc(), down from 0x4a4 bytes to 0x74 bytes (x86-32). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 137 ++++++++++++++++++++++++++------------------ 1 files changed, 81 insertions(+), 56 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-15 15:32:42.000000000 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,86 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} - void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + u_short *tbuf; + cisparse_t *cisparse; + config_info_t *cfginfo = 0; + cistpl_cftable_entry_t *cfg; + cistpl_cftable_entry_t *def_cte = 0; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); + if (!tbuf) goto err_tbuf; + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); + if (!def_cte) goto err_def_cte; + memset(def_cte, 0, sizeof(*def_cte)); + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); + if (!cfginfo) goto err_cfginfo; + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); + if (!cisparse) goto err_cisparse; + cfg = &cisparse->cftable_entry; + + tuple.TupleData = (cisdata_t *)tbuf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, cisparse)); + link->conf.ConfigBase = cisparse->config.base; + link->conf.Present = cisparse->config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, cisparse)) + is_kme = ((cisparse->manfid.manf == MANFID_KME) && + ((cisparse->manfid.card == PRODID_KME_KXLC005_A) || + (cisparse->manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, cfginfo)); + link->conf.Vcc = cfginfo->Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, cisparse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (def_cte->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != def_cte->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (def_cte->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + def_cte->vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (def_cte->io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &def_cte->io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +319,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(def_cte, cfg, sizeof(*def_cte)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(def_cte, 0, sizeof(*def_cte)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +349,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +378,34 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; return; - + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: ide_release(link); link->state &= ~DEV_CONFIG_PENDING; + /* memory allocation errors */ +err_cisparse: + kfree(cfginfo); +err_cfginfo: + kfree(def_cte); +err_def_cte: + kfree(tbuf); +err_tbuf: + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] >3k call path in ide 2004-06-15 23:34 ` [PATCH] " Randy.Dunlap @ 2004-06-16 7:11 ` Florian Schirmer 2004-06-16 9:47 ` Jörn Engel 0 siblings, 1 reply; 14+ messages in thread From: Florian Schirmer @ 2004-06-16 7:11 UTC (permalink / raw) To: Randy.Dunlap Cc: Jörn Engel, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel Hi, > failed: > ide_release(link); > link->state &= ~DEV_CONFIG_PENDING; > > + /* memory allocation errors */ > +err_cisparse: > + kfree(cfginfo); > +err_cfginfo: > + kfree(def_cte); > +err_def_cte: > + kfree(tbuf); > +err_tbuf: > + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); > + goto failed; > } /* ide_config */ Huh? This will either leak memory (non alloc error case) or deadlock (mem alloc error case). I'm missing something? Best, Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] >3k call path in ide 2004-06-16 7:11 ` Florian Schirmer @ 2004-06-16 9:47 ` Jörn Engel 2004-06-16 9:55 ` Florian Schirmer 0 siblings, 1 reply; 14+ messages in thread From: Jörn Engel @ 2004-06-16 9:47 UTC (permalink / raw) To: Florian Schirmer Cc: Randy.Dunlap, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 June 2004 09:11:11 +0200, Florian Schirmer wrote: > > > failed: > > ide_release(link); > > link->state &= ~DEV_CONFIG_PENDING; > > > > + /* memory allocation errors */ > > +err_cisparse: > > + kfree(cfginfo); > > +err_cfginfo: > > + kfree(def_cte); > > +err_def_cte: > > + kfree(tbuf); > > +err_tbuf: > > + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); > > + goto failed; > > } /* ide_config */ > > Huh? This will either leak memory (non alloc error case) or deadlock (mem > alloc error case). I'm missing something? Leak memory. I also tend to depend on the fact that kfree(NULL) works just fine: err_kfree: kfree(cfginfo); kfree(def_cte); kfree(tbuf); printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); goto failed; Makes the error path a little simpler. Jörn -- The strong give up and move away, while the weak give up and stay. -- unknown ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] >3k call path in ide 2004-06-16 9:47 ` Jörn Engel @ 2004-06-16 9:55 ` Florian Schirmer 2004-06-16 10:00 ` Jörn Engel 0 siblings, 1 reply; 14+ messages in thread From: Florian Schirmer @ 2004-06-16 9:55 UTC (permalink / raw) To: Jörn Engel Cc: Randy.Dunlap, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel Hi, >Leak memory. I also tend to depend on the fact that kfree(NULL) works >just fine: > >err_kfree: > kfree(cfginfo); > kfree(def_cte); > kfree(tbuf); > printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); > goto failed; > >Makes the error path a little simpler. > > Nope. It will deadlock just like the original patch because failed falls through to err_kfree which then will jump to failed... Regards, Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] >3k call path in ide 2004-06-16 9:55 ` Florian Schirmer @ 2004-06-16 10:00 ` Jörn Engel 2004-06-16 17:37 ` [PATCH] [STACK] reduce " Randy.Dunlap 0 siblings, 1 reply; 14+ messages in thread From: Jörn Engel @ 2004-06-16 10:00 UTC (permalink / raw) To: Florian Schirmer Cc: Randy.Dunlap, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 June 2004 11:55:52 +0200, Florian Schirmer wrote: > > >Leak memory. > > Nope. It will deadlock just like the original patch because failed falls > through to err_kfree which then will jump to failed... Both, leak memory on good path, deadlock on error path. Fun patch. :) Jörn -- It is better to die of hunger having lived without grief and fear, than to live with a troubled spirit amid abundance. -- Epictetus ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 10:00 ` Jörn Engel @ 2004-06-16 17:37 ` Randy.Dunlap 2004-06-16 17:57 ` Jörn Engel 2004-06-16 18:58 ` Brian Gerst 0 siblings, 2 replies; 14+ messages in thread From: Randy.Dunlap @ 2004-06-16 17:37 UTC (permalink / raw) To: Jörn Engel; +Cc: jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel Thanks for the helpful comments. Here's a corrected patch. Reduce large stack usage in ide_config() by using kmalloc(). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 135 +++++++++++++++++++++++++------------------- 1 files changed, 79 insertions(+), 56 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-16 09:11:14.431098048 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,86 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} - void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + u_short *tbuf = 0; + cisparse_t *cisparse = 0; + config_info_t *cfginfo = 0; + cistpl_cftable_entry_t *cfg; + cistpl_cftable_entry_t *def_cte = 0; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); + if (!tbuf) goto err_kfree; + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); + if (!def_cte) goto err_kfree; + memset(def_cte, 0, sizeof(*def_cte)); + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); + if (!cfginfo) goto err_kfree; + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); + if (!cisparse) goto err_kfree; + cfg = &cisparse->cftable_entry; + + tuple.TupleData = (cisdata_t *)tbuf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, cisparse)); + link->conf.ConfigBase = cisparse->config.base; + link->conf.Present = cisparse->config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, cisparse)) + is_kme = ((cisparse->manfid.manf == MANFID_KME) && + ((cisparse->manfid.card == PRODID_KME_KXLC005_A) || + (cisparse->manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, cfginfo)); + link->conf.Vcc = cfginfo->Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, cisparse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (def_cte->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != def_cte->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (def_cte->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + def_cte->vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (def_cte->io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &def_cte->io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +319,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(def_cte, cfg, sizeof(*def_cte)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(def_cte, 0, sizeof(*def_cte)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +349,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +378,32 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; return; - + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: ide_release(link); link->state &= ~DEV_CONFIG_PENDING; + return; + /* memory allocation errors */ +err_kfree: + kfree(cfginfo); + kfree(def_cte); + kfree(tbuf); + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 17:37 ` [PATCH] [STACK] reduce " Randy.Dunlap @ 2004-06-16 17:57 ` Jörn Engel 2004-06-16 18:16 ` Randy.Dunlap 2004-06-16 18:58 ` Brian Gerst 1 sibling, 1 reply; 14+ messages in thread From: Jörn Engel @ 2004-06-16 17:57 UTC (permalink / raw) To: Randy.Dunlap; +Cc: jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 June 2004 10:37:41 -0700, Randy.Dunlap wrote: > > Thanks for the helpful comments. Here's a corrected patch. Looks, as if it still leaks memory: > > link->state &= ~DEV_CONFIG_PENDING; about here. > return; > - > + > cs_failed: > cs_error(link->handle, last_fn, last_ret); > failed: > ide_release(link); > link->state &= ~DEV_CONFIG_PENDING; > + return; > > + /* memory allocation errors */ > +err_kfree: > + kfree(cfginfo); > + kfree(def_cte); > + kfree(tbuf); > + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); > + goto failed; > } /* ide_config */ > > /*====================================================================== Jörn -- Schrödinger's cat is <BLINK>not</BLINK> dead. -- Illiad ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 17:57 ` Jörn Engel @ 2004-06-16 18:16 ` Randy.Dunlap 2004-06-16 18:29 ` Jörn Engel 0 siblings, 1 reply; 14+ messages in thread From: Randy.Dunlap @ 2004-06-16 18:16 UTC (permalink / raw) To: Jörn Engel; +Cc: jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 Jun 2004 19:57:30 +0200 Jörn Engel wrote: | On Wed, 16 June 2004 10:37:41 -0700, Randy.Dunlap wrote: | > | > Thanks for the helpful comments. Here's a corrected patch. | | Looks, as if it still leaks memory: duh. fudge. Thanks. How's this one? Reduce large stack usage in ide_config() by using kmalloc(), down from 0x4a4 bytes to 0x74 bytes (x86-32). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 137 +++++++++++++++++++++++++------------------- 1 files changed, 80 insertions(+), 57 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-16 10:42:00.554161496 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,86 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} - void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + u_short *tbuf = 0; + cisparse_t *cisparse = 0; + config_info_t *cfginfo = 0; + cistpl_cftable_entry_t *cfg; + cistpl_cftable_entry_t *def_cte = 0; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); + if (!tbuf) goto err_mem; + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); + if (!def_cte) goto err_mem; + memset(def_cte, 0, sizeof(*def_cte)); + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); + if (!cfginfo) goto err_mem; + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); + if (!cisparse) goto err_mem; + cfg = &cisparse->cftable_entry; + + tuple.TupleData = (cisdata_t *)tbuf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, cisparse)); + link->conf.ConfigBase = cisparse->config.base; + link->conf.Present = cisparse->config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, cisparse)) + is_kme = ((cisparse->manfid.manf == MANFID_KME) && + ((cisparse->manfid.card == PRODID_KME_KXLC005_A) || + (cisparse->manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, cfginfo)); + link->conf.Vcc = cfginfo->Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, cisparse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (def_cte->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != def_cte->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (def_cte->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + def_cte->vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (def_cte->io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &def_cte->io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +319,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(def_cte, cfg, sizeof(*def_cte)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(def_cte, 0, sizeof(*def_cte)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +349,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +378,32 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; return; - + +err_mem: + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: + kfree(cisparse); + kfree(cfginfo); + kfree(def_cte); + kfree(tbuf); + ide_release(link); link->state &= ~DEV_CONFIG_PENDING; - } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 18:16 ` Randy.Dunlap @ 2004-06-16 18:29 ` Jörn Engel 2004-06-16 18:49 ` Randy.Dunlap 0 siblings, 1 reply; 14+ messages in thread From: Jörn Engel @ 2004-06-16 18:29 UTC (permalink / raw) To: Randy.Dunlap; +Cc: jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 June 2004 11:16:21 -0700, Randy.Dunlap wrote: > On Wed, 16 Jun 2004 19:57:30 +0200 Jörn Engel wrote: > > | On Wed, 16 June 2004 10:37:41 -0700, Randy.Dunlap wrote: > | > > | > Thanks for the helpful comments. Here's a corrected patch. > | > | Looks, as if it still leaks memory: > > duh. fudge. Thanks. How's this one? Just four more lines? > link->dev = &info->node; > printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", > - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, > - link->conf.Vpp1/10, link->conf.Vpp1%10); > + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, > + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); > > link->state &= ~DEV_CONFIG_PENDING; + kfree(cisparse); + kfree(cfginfo); + kfree(def_cte); + kfree(tbuf); > return; > - > + > +err_mem: > + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); > + goto failed; > + > cs_failed: > cs_error(link->handle, last_fn, last_ret); > failed: > + kfree(cisparse); > + kfree(cfginfo); > + kfree(def_cte); > + kfree(tbuf); > + > ide_release(link); > link->state &= ~DEV_CONFIG_PENDING; > - > } /* ide_config */ > > /*====================================================================== Jörn -- Fancy algorithms are slow when n is small, and n is usually small. Fancy algorithms have big constants. Until you know that n is frequently going to be big, don't get fancy. -- Rob Pike ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 18:29 ` Jörn Engel @ 2004-06-16 18:49 ` Randy.Dunlap 0 siblings, 0 replies; 14+ messages in thread From: Randy.Dunlap @ 2004-06-16 18:49 UTC (permalink / raw) To: Jörn Engel; +Cc: jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 Jun 2004 20:29:10 +0200 Jörn Engel wrote: | On Wed, 16 June 2004 11:16:21 -0700, Randy.Dunlap wrote: | > On Wed, 16 Jun 2004 19:57:30 +0200 Jörn Engel wrote: | > | > | On Wed, 16 June 2004 10:37:41 -0700, Randy.Dunlap wrote: | > | > | > | > Thanks for the helpful comments. Here's a corrected patch. | > | | > | Looks, as if it still leaks memory: | > | > duh. fudge. Thanks. How's this one? | | Just four more lines? OK, Randy, slow down and go to lunch. Thanks, Jörn. Reduce large stack usage in ide_config() by using kmalloc(), down from 0x4a4 bytes to 0x74 bytes (x86-32). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 141 ++++++++++++++++++++++++++------------------ 1 files changed, 84 insertions(+), 57 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-16 11:01:43.000000000 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,86 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} - void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + u_short *tbuf = 0; + cisparse_t *cisparse = 0; + config_info_t *cfginfo = 0; + cistpl_cftable_entry_t *cfg; + cistpl_cftable_entry_t *def_cte = 0; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); + if (!tbuf) goto err_mem; + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); + if (!def_cte) goto err_mem; + memset(def_cte, 0, sizeof(*def_cte)); + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); + if (!cfginfo) goto err_mem; + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); + if (!cisparse) goto err_mem; + cfg = &cisparse->cftable_entry; + + tuple.TupleData = (cisdata_t *)tbuf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, cisparse)); + link->conf.ConfigBase = cisparse->config.base; + link->conf.Present = cisparse->config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, cisparse)) + is_kme = ((cisparse->manfid.manf == MANFID_KME) && + ((cisparse->manfid.card == PRODID_KME_KXLC005_A) || + (cisparse->manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, cfginfo)); + link->conf.Vcc = cfginfo->Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, cisparse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (def_cte->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (cfginfo->Vcc != def_cte->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (def_cte->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + def_cte->vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (def_cte->io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &def_cte->io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +319,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(def_cte, cfg, sizeof(*def_cte)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(def_cte, 0, sizeof(*def_cte)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +349,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +378,36 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; + kfree(cisparse); + kfree(cfginfo); + kfree(def_cte); + kfree(tbuf); return; - + +err_mem: + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: + kfree(cisparse); + kfree(cfginfo); + kfree(def_cte); + kfree(tbuf); + ide_release(link); link->state &= ~DEV_CONFIG_PENDING; - } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 17:37 ` [PATCH] [STACK] reduce " Randy.Dunlap 2004-06-16 17:57 ` Jörn Engel @ 2004-06-16 18:58 ` Brian Gerst 2004-06-16 19:52 ` Randy.Dunlap 1 sibling, 1 reply; 14+ messages in thread From: Brian Gerst @ 2004-06-16 18:58 UTC (permalink / raw) To: Randy.Dunlap Cc: Jörn Engel, jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel Randy.Dunlap wrote: > + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); > + if (!tbuf) goto err_kfree; > + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); > + if (!def_cte) goto err_kfree; > + memset(def_cte, 0, sizeof(*def_cte)); > + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); > + if (!cfginfo) goto err_kfree; > + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); > + if (!cisparse) goto err_kfree; This can be condensed into a single kmalloc. Define a struct that contains these variables and kmalloc the whole struct in one call. -- Brian Gerst ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 18:58 ` Brian Gerst @ 2004-06-16 19:52 ` Randy.Dunlap 2004-06-16 22:53 ` Randy.Dunlap 0 siblings, 1 reply; 14+ messages in thread From: Randy.Dunlap @ 2004-06-16 19:52 UTC (permalink / raw) To: Brian Gerst; +Cc: joern, jolt, akpm, B.Zolnierkiewicz, linux-ide, linux-kernel On Wed, 16 Jun 2004 14:58:19 -0400 Brian Gerst wrote: | Randy.Dunlap wrote: | > + tbuf = kmalloc(128 * sizeof(u_short), GFP_KERNEL); | > + if (!tbuf) goto err_kfree; | > + def_cte = kmalloc(sizeof(*def_cte), GFP_KERNEL); | > + if (!def_cte) goto err_kfree; | > + memset(def_cte, 0, sizeof(*def_cte)); | > + cfginfo = kmalloc(sizeof(*cfginfo), GFP_KERNEL); | > + if (!cfginfo) goto err_kfree; | > + cisparse = kmalloc(sizeof(*cisparse), GFP_KERNEL); | > + if (!cisparse) goto err_kfree; | | This can be condensed into a single kmalloc. Define a struct that | contains these variables and kmalloc the whole struct in one call. OK, how's this one? and thanks. Reduce large stack usage in ide_config() by using kmalloc(), down from 0x4a4 bytes to 0x64 bytes (x86-32). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 130 +++++++++++++++++++++++++------------------- 1 files changed, 74 insertions(+), 56 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-16 11:59:58.728659752 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,84 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} +struct stk { + u_short buf[128]; + cisparse_t parse; + config_info_t conf; + cistpl_cftable_entry_t dflt; +}; void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + struct stk *stk = 0; + cistpl_cftable_entry_t *cfg; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + stk = kmalloc(sizeof(*stk), GFP_KERNEL); + if (!stk) goto err_mem; + memset(stk, 0, sizeof(*stk)); + cfg = &stk->parse.cftable_entry; + + tuple.TupleData = (cisdata_t *)&stk->buf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &stk->parse)); + link->conf.ConfigBase = stk->parse.config.base; + link->conf.Present = stk->parse.config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, &stk->parse)) + is_kme = ((stk->parse.manfid.manf == MANFID_KME) && + ((stk->parse.manfid.card == PRODID_KME_KXLC005_A) || + (stk->parse.manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &stk->conf)); + link->conf.Vcc = stk->conf.Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, &stk->parse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (stk->conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (stk->dflt.vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (stk->conf.Vcc != stk->dflt.vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (stk->dflt.vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + stk->dflt.vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (stk->dflt.io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &stk->dflt.io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +317,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(&stk->dflt, cfg, sizeof(stk->dflt)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(&stk->dflt, 0, sizeof(stk->dflt)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +347,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +376,29 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; + kfree(stk); return; - + +err_mem: + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: + kfree(stk); ide_release(link); link->state &= ~DEV_CONFIG_PENDING; - } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] [STACK] reduce >3k call path in ide 2004-06-16 19:52 ` Randy.Dunlap @ 2004-06-16 22:53 ` Randy.Dunlap 0 siblings, 0 replies; 14+ messages in thread From: Randy.Dunlap @ 2004-06-16 22:53 UTC (permalink / raw) To: lkml; +Cc: bgerst, joern, jolt, akpm, B.Zolnierkiewicz, linux-ide This version moves the stack structure "stk" inside the ide_config() function as an anonymous struct. I hope this is the last one.... :) Reduce large stack usage in ide_config() by using kmalloc(), down from 0x4a4 bytes to 0x64 bytes (x86-32). Little whitespace cleanup. Move function comment block to immediately above the function. Module loaded and unloaded, otherwise not tested (no hardware). Signed-off-by: Randy Dunlap <rddunlap@osdl.org> diffstat:= drivers/ide/legacy/ide-cs.c | 130 ++++++++++++++++++++++++-------------------- 1 files changed, 73 insertions(+), 57 deletions(-) diff -Naurp ./drivers/ide/legacy/ide-cs.c~idecs_stack ./drivers/ide/legacy/ide-cs.c --- ./drivers/ide/legacy/ide-cs.c~idecs_stack 2004-05-09 19:32:53.000000000 -0700 +++ ./drivers/ide/legacy/ide-cs.c 2004-06-16 13:31:08.285160776 -0700 @@ -199,6 +199,16 @@ static void ide_detach(dev_link_t *link) } /* ide_detach */ +static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) +{ + hw_regs_t hw; + memset(&hw, 0, sizeof(hw)); + ide_init_hwif_ports(&hw, io, ctl, NULL); + hw.irq = irq; + hw.chipset = ide_pci; + return ide_register_hw(&hw, NULL); +} + /*====================================================================== ide_config() is scheduled to run after a CARD_INSERTION event @@ -210,84 +220,82 @@ static void ide_detach(dev_link_t *link) #define CS_CHECK(fn, ret) \ do { last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; } while (0) -static int idecs_register(unsigned long io, unsigned long ctl, unsigned long irq) -{ - hw_regs_t hw; - memset(&hw, 0, sizeof(hw)); - ide_init_hwif_ports(&hw, io, ctl, NULL); - hw.irq = irq; - hw.chipset = ide_pci; - return ide_register_hw(&hw, NULL); -} - void ide_config(dev_link_t *link) { client_handle_t handle = link->handle; ide_info_t *info = link->priv; tuple_t tuple; - u_short buf[128]; - cisparse_t parse; - config_info_t conf; - cistpl_cftable_entry_t *cfg = &parse.cftable_entry; - cistpl_cftable_entry_t dflt = { 0 }; - int i, pass, last_ret, last_fn, hd, is_kme = 0; + struct { + u_short buf[128]; + cisparse_t parse; + config_info_t conf; + cistpl_cftable_entry_t dflt; + } *stk = 0; + cistpl_cftable_entry_t *cfg; + int i, pass, last_ret = 0, last_fn = 0, hd, is_kme = 0; unsigned long io_base, ctl_base; DEBUG(0, "ide_config(0x%p)\n", link); - - tuple.TupleData = (cisdata_t *)buf; - tuple.TupleOffset = 0; tuple.TupleDataMax = 255; + + stk = kmalloc(sizeof(*stk), GFP_KERNEL); + if (!stk) goto err_mem; + memset(stk, 0, sizeof(*stk)); + cfg = &stk->parse.cftable_entry; + + tuple.TupleData = (cisdata_t *)&stk->buf; + tuple.TupleOffset = 0; + tuple.TupleDataMax = 255; tuple.Attributes = 0; tuple.DesiredTuple = CISTPL_CONFIG; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); CS_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple)); - CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse)); - link->conf.ConfigBase = parse.config.base; - link->conf.Present = parse.config.rmask[0]; + CS_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &stk->parse)); + link->conf.ConfigBase = stk->parse.config.base; + link->conf.Present = stk->parse.config.rmask[0]; tuple.DesiredTuple = CISTPL_MANFID; if (!pcmcia_get_first_tuple(handle, &tuple) && !pcmcia_get_tuple_data(handle, &tuple) && - !pcmcia_parse_tuple(handle, &tuple, &parse)) - is_kme = ((parse.manfid.manf == MANFID_KME) && - ((parse.manfid.card == PRODID_KME_KXLC005_A) || - (parse.manfid.card == PRODID_KME_KXLC005_B))); + !pcmcia_parse_tuple(handle, &tuple, &stk->parse)) + is_kme = ((stk->parse.manfid.manf == MANFID_KME) && + ((stk->parse.manfid.card == PRODID_KME_KXLC005_A) || + (stk->parse.manfid.card == PRODID_KME_KXLC005_B))); /* Configure card */ link->state |= DEV_CONFIG; /* Not sure if this is right... look up the current Vcc */ - CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &conf)); - link->conf.Vcc = conf.Vcc; - + CS_CHECK(GetConfigurationInfo, pcmcia_get_configuration_info(handle, &stk->conf)); + link->conf.Vcc = stk->conf.Vcc; + pass = io_base = ctl_base = 0; tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY; tuple.Attributes = 0; CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); while (1) { if (pcmcia_get_tuple_data(handle, &tuple) != 0) goto next_entry; - if (pcmcia_parse_tuple(handle, &tuple, &parse) != 0) goto next_entry; + if (pcmcia_parse_tuple(handle, &tuple, &stk->parse) != 0) goto next_entry; /* Check for matching Vcc, unless we're desperate */ if (!pass) { - if (cfg->vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM]/10000) + if (cfg->vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (stk->conf.Vcc != cfg->vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; - } else if (dflt.vcc.present & (1<<CISTPL_POWER_VNOM)) { - if (conf.Vcc != dflt.vcc.param[CISTPL_POWER_VNOM]/10000) + } else if (stk->dflt.vcc.present & (1 << CISTPL_POWER_VNOM)) { + if (stk->conf.Vcc != stk->dflt.vcc.param[CISTPL_POWER_VNOM] / 10000) goto next_entry; } } - - if (cfg->vpp1.present & (1<<CISTPL_POWER_VNOM)) + + if (cfg->vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - cfg->vpp1.param[CISTPL_POWER_VNOM]/10000; - else if (dflt.vpp1.present & (1<<CISTPL_POWER_VNOM)) + cfg->vpp1.param[CISTPL_POWER_VNOM] / 10000; + else if (stk->dflt.vpp1.present & (1 << CISTPL_POWER_VNOM)) link->conf.Vpp1 = link->conf.Vpp2 = - dflt.vpp1.param[CISTPL_POWER_VNOM]/10000; - - if ((cfg->io.nwin > 0) || (dflt.io.nwin > 0)) { - cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &dflt.io; + stk->dflt.vpp1.param[CISTPL_POWER_VNOM] / 10000; + + if ((cfg->io.nwin > 0) || (stk->dflt.io.nwin > 0)) { + cistpl_io_t *io = (cfg->io.nwin) ? &cfg->io : &stk->dflt.io; link->conf.ConfigIndex = cfg->index; link->io.BasePort1 = io->win[0].base; link->io.IOAddrLines = io->flags & CISTPL_IO_LINES_MASK; @@ -307,23 +315,24 @@ void ide_config(dev_link_t *link) if (pcmcia_request_io(link->handle, &link->io) != 0) goto next_entry; io_base = link->io.BasePort1; - ctl_base = link->io.BasePort1+0x0e; + ctl_base = link->io.BasePort1 + 0x0e; } else goto next_entry; /* If we've got this far, we're done */ break; } - + next_entry: - if (cfg->flags & CISTPL_CFTABLE_DEFAULT) dflt = *cfg; + if (cfg->flags & CISTPL_CFTABLE_DEFAULT) + memcpy(&stk->dflt, cfg, sizeof(stk->dflt)); if (pass) { CS_CHECK(GetNextTuple, pcmcia_get_next_tuple(handle, &tuple)); } else if (pcmcia_get_next_tuple(handle, &tuple) != 0) { CS_CHECK(GetFirstTuple, pcmcia_get_first_tuple(handle, &tuple)); - memset(&dflt, 0, sizeof(dflt)); + memset(&stk->dflt, 0, sizeof(stk->dflt)); pass++; } } - + CS_CHECK(RequestIRQ, pcmcia_request_irq(handle, &link->irq)); CS_CHECK(RequestConfiguration, pcmcia_request_configuration(handle, &link->conf)); @@ -336,25 +345,27 @@ void ide_config(dev_link_t *link) outb(0x02, ctl_base); /* special setup for KXLC005 card */ - if (is_kme) outb(0x81, ctl_base+1); + if (is_kme) + outb(0x81, ctl_base+1); /* retry registration in case device is still spinning up */ for (hd = -1, i = 0; i < 10; i++) { hd = idecs_register(io_base, ctl_base, link->irq.AssignedIRQ); if (hd >= 0) break; if (link->io.NumPorts1 == 0x20) { - outb(0x02, ctl_base+0x10); - hd = idecs_register(io_base+0x10, ctl_base+0x10, + outb(0x02, ctl_base + 0x10); + hd = idecs_register(io_base + 0x10, ctl_base + 0x10, link->irq.AssignedIRQ); if (hd >= 0) { - io_base += 0x10; ctl_base += 0x10; + io_base += 0x10; + ctl_base += 0x10; break; } } __set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(HZ/10); } - + if (hd < 0) { printk(KERN_NOTICE "ide-cs: ide_register() at 0x%3lx & 0x%3lx" ", irq %u failed\n", io_base, ctl_base, @@ -363,24 +374,29 @@ void ide_config(dev_link_t *link) } info->ndev = 1; - sprintf(info->node.dev_name, "hd%c", 'a'+(hd*2)); + sprintf(info->node.dev_name, "hd%c", 'a' + (hd * 2)); info->node.major = ide_major[hd]; info->node.minor = 0; info->hd = hd; link->dev = &info->node; printk(KERN_INFO "ide-cs: %s: Vcc = %d.%d, Vpp = %d.%d\n", - info->node.dev_name, link->conf.Vcc/10, link->conf.Vcc%10, - link->conf.Vpp1/10, link->conf.Vpp1%10); + info->node.dev_name, link->conf.Vcc / 10, link->conf.Vcc % 10, + link->conf.Vpp1 / 10, link->conf.Vpp1 % 10); link->state &= ~DEV_CONFIG_PENDING; + kfree(stk); return; - + +err_mem: + printk(KERN_NOTICE "ide-cs: ide_config failed memory allocation\n"); + goto failed; + cs_failed: cs_error(link->handle, last_fn, last_ret); failed: + kfree(stk); ide_release(link); link->state &= ~DEV_CONFIG_PENDING; - } /* ide_config */ /*====================================================================== ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2004-06-16 22:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-06-09 12:29 [STACK] >3k call path in ide Jörn Engel 2004-06-15 23:34 ` [PATCH] " Randy.Dunlap 2004-06-16 7:11 ` Florian Schirmer 2004-06-16 9:47 ` Jörn Engel 2004-06-16 9:55 ` Florian Schirmer 2004-06-16 10:00 ` Jörn Engel 2004-06-16 17:37 ` [PATCH] [STACK] reduce " Randy.Dunlap 2004-06-16 17:57 ` Jörn Engel 2004-06-16 18:16 ` Randy.Dunlap 2004-06-16 18:29 ` Jörn Engel 2004-06-16 18:49 ` Randy.Dunlap 2004-06-16 18:58 ` Brian Gerst 2004-06-16 19:52 ` Randy.Dunlap 2004-06-16 22:53 ` Randy.Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).