* [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).