* [STACK] >3k call path in ide
@ 2004-06-09 12:29 Jörn Engel
2004-06-09 15:23 ` Michael Clark
2004-06-15 23:34 ` [PATCH] " Randy.Dunlap
0 siblings, 2 replies; 21+ 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] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-09 12:29 [STACK] >3k call path in ide Jörn Engel
@ 2004-06-09 15:23 ` Michael Clark
2004-06-09 16:29 ` Jörn Engel
2004-06-15 23:34 ` [PATCH] " Randy.Dunlap
1 sibling, 1 reply; 21+ messages in thread
From: Michael Clark @ 2004-06-09 15:23 UTC (permalink / raw)
To: Jörn Engel, linux-kernel
Hi Jorn,
Noticed that try_to_free_pages, sync_inodes_sb and wakeup_bdflush features
in almost all of these traces and although at 284, 308 and 256 respectively
their not huge but together their neither that small (considering they
occur all in the same stack trace).
This is consumed mostly by a struct page_state which is 148 bytes big
although looking at the code get_page_state(struct page_state *ret)
only populates the first 6 fields or 24 bytes. get_full_page_state
which is hardly used updates these other fields.
Is this a candidate for splitting into 2 structs? 1 containing just the
first 6 fields needed by the majority of users: try_to_free_pages,
shrink_all_memory, kswapd, get_dirty_limits, wakeup_bdflush, sync_inodes_sb
mclark@monty:/usr/src/linux$ find . -name '*.c' | xargs grep get_page_state
./fs/proc/proc_misc.c: get_page_state(&ps);
./fs/fs-writeback.c: get_page_state(&ps);
./mm/page_alloc.c:void __get_page_state(struct page_state *ret, int nr)
./mm/page_alloc.c:void get_page_state(struct page_state *ret)
./mm/page_alloc.c: __get_page_state(ret, nr + 1);
./mm/page_alloc.c: __get_page_state(ret, sizeof(*ret) / sizeof(unsigned long));
./mm/page_alloc.c: get_page_state(&ps);
./mm/vmscan.c: get_page_state(&ps);
./mm/vmscan.c: get_page_state(&ps);
./mm/vmscan.c: get_page_state(&ps);
./mm/page-writeback.c: get_page_state(ps);
./mm/page-writeback.c: * On really big machines, get_page_state is expensive, so try to avoid calling
./mm/page-writeback.c: get_page_state(&ps);
./mm/page-writeback.c: get_page_state(&ps);
./mm/page-writeback.c: * If it is too low then SMP machines will call the (expensive) get_page_state
mclark@monty:/usr/src/linux$ find . -name '*.c' | xargs grep get_full_page_state
./drivers/parisc/led.c: get_full_page_state(&pgstat); /* get no of sectors in & out */
./arch/s390/appldata/appldata_base.c:void get_full_page_state(struct page_state *ps)
./arch/s390/appldata/appldata_base.c:EXPORT_SYMBOL_GPL(get_full_page_state);
./arch/s390/appldata/appldata_mem.c: get_full_page_state(&ps);
./mm/page_alloc.c:void get_full_page_state(struct page_state *ret)
./mm/page_alloc.c: get_full_page_state(ps);
~mc
On 06/09/04 20:29, 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
> 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
>
--
Michael Clark, . . . . . . . . . . . . michael@metaparadigm.com
Managing Director, . . . . . . . . . http://www.metaparadigm.com
Metaparadigm Pte. Ltd. . . . . . . . . . . phone: +65 6395 6277
25F Paterson Road, . . . . . . . . . . . . mobile: +65 9645 9612
Singapore 238515 . . . . . . . . . . . . . . fax: +65 6234 4043
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-09 15:23 ` Michael Clark
@ 2004-06-09 16:29 ` Jörn Engel
2004-06-09 19:27 ` Andrew Morton
0 siblings, 1 reply; 21+ messages in thread
From: Jörn Engel @ 2004-06-09 16:29 UTC (permalink / raw)
To: Michael Clark; +Cc: linux-kernel, Hugh Dickins, Andrew Morton
On Wed, 9 June 2004 23:23:20 +0800, Michael Clark wrote:
>
> Noticed that try_to_free_pages, sync_inodes_sb and wakeup_bdflush features
> in almost all of these traces and although at 284, 308 and 256 respectively
> their not huge but together their neither that small (considering they
> occur all in the same stack trace).
>
> This is consumed mostly by a struct page_state which is 148 bytes big
> although looking at the code get_page_state(struct page_state *ret)
> only populates the first 6 fields or 24 bytes. get_full_page_state
> which is hardly used updates these other fields.
>
> Is this a candidate for splitting into 2 structs? 1 containing just the
> first 6 fields needed by the majority of users: try_to_free_pages,
> shrink_all_memory, kswapd, get_dirty_limits, wakeup_bdflush, sync_inodes_sb
Well noticed, although Hugh and Andrew have already exchanged some
patches, see
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0406.1/0134.html
For the moment I consider try_to_free_pages() fixed.
Andrew, what do you thing about the patch below for sync_inodes_sb()?
It's stack consumption is reduced from 308 to 64, at the cost of one
more function call.
Jörn
--
Premature optimization is the root of all evil.
-- Donald Knuth
Move a struct page_state into it's own function. This reduces the stack
consumption for sync_inodes_sb(), as the stack is already partially rolled
back before other functions get called.
Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
fs-writeback.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
--- linux-2.6.6cow/fs/fs-writeback.c~sync_inodes_sb 2004-06-09 18:19:25.000000000 +0200
+++ linux-2.6.6cow/fs/fs-writeback.c 2004-06-09 18:23:44.000000000 +0200
@@ -396,6 +396,15 @@
spin_unlock(&inode_lock);
}
+static long get_nr_to_write(void)
+{
+ struct page_state ps;
+
+ get_page_state(&ps);
+ return ps.nr_dirty + ps.nr_unstable + ps.nr_dirty + ps.nr_unstable +
+ (inodes_stat.nr_inodes - inodes_stat.nr_unused);
+}
+
/*
* writeback and wait upon the filesystem's dirty inodes. The caller will
* do this in two passes - one to write, and one to wait. WB_SYNC_HOLD is
@@ -409,7 +418,6 @@
*/
void sync_inodes_sb(struct super_block *sb, int wait)
{
- struct page_state ps;
struct writeback_control wbc = {
.bdi = NULL,
.sync_mode = wait ? WB_SYNC_ALL : WB_SYNC_HOLD,
@@ -417,10 +425,7 @@
.nr_to_write = 0,
};
- get_page_state(&ps);
- wbc.nr_to_write = ps.nr_dirty + ps.nr_unstable +
- (inodes_stat.nr_inodes - inodes_stat.nr_unused) +
- ps.nr_dirty + ps.nr_unstable;
+ wbc.nr_to_write = get_nr_to_write();
wbc.nr_to_write += wbc.nr_to_write / 2; /* Bit more for luck */
spin_lock(&inode_lock);
sync_sb_inodes(sb, &wbc);
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-09 16:29 ` Jörn Engel
@ 2004-06-09 19:27 ` Andrew Morton
2004-06-10 22:59 ` Jörn Engel
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2004-06-09 19:27 UTC (permalink / raw)
To: Jörn Engel; +Cc: michael, linux-kernel, hugh
Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:
>
> Andrew, what do you thing about the patch below for sync_inodes_sb()?
> It's stack consumption is reduced from 308 to 64, at the cost of one
> more function call.
Like this:
--- 25/fs/fs-writeback.c~sync_inodes_sb-stack-reduction 2004-06-09 12:25:57.111389456 -0700
+++ 25-akpm/fs/fs-writeback.c 2004-06-09 12:25:57.115388848 -0700
@@ -433,15 +433,15 @@ restart:
*/
void sync_inodes_sb(struct super_block *sb, int wait)
{
- struct page_state ps;
struct writeback_control wbc = {
.sync_mode = wait ? WB_SYNC_ALL : WB_SYNC_HOLD,
};
+ unsigned long nr_dirty = read_page_state(nr_dirty);
+ unsigned long nr_unstable = read_page_state(nr_unstable);
- get_page_state(&ps);
- wbc.nr_to_write = ps.nr_dirty + ps.nr_unstable +
+ wbc.nr_to_write = nr_dirty + nr_unstable +
(inodes_stat.nr_inodes - inodes_stat.nr_unused) +
- ps.nr_dirty + ps.nr_unstable;
+ nr_dirty + nr_unstable;
wbc.nr_to_write += wbc.nr_to_write / 2; /* Bit more for luck */
spin_lock(&inode_lock);
sync_sb_inodes(sb, &wbc);
_
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-09 19:27 ` Andrew Morton
@ 2004-06-10 22:59 ` Jörn Engel
2004-06-10 23:10 ` Andrew Morton
0 siblings, 1 reply; 21+ messages in thread
From: Jörn Engel @ 2004-06-10 22:59 UTC (permalink / raw)
To: Andrew Morton; +Cc: michael, linux-kernel, hugh
On Wed, 9 June 2004 12:27:21 -0700, Andrew Morton wrote:
> Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:
> >
> > Andrew, what do you thing about the patch below for sync_inodes_sb()?
> > It's stack consumption is reduced from 308 to 64, at the cost of one
> > more function call.
>
> Like this:
>
> --- 25/fs/fs-writeback.c~sync_inodes_sb-stack-reduction 2004-06-09 12:25:57.111389456 -0700
> +++ 25-akpm/fs/fs-writeback.c 2004-06-09 12:25:57.115388848 -0700
> @@ -433,15 +433,15 @@ restart:
> */
> void sync_inodes_sb(struct super_block *sb, int wait)
> {
> - struct page_state ps;
> struct writeback_control wbc = {
> .sync_mode = wait ? WB_SYNC_ALL : WB_SYNC_HOLD,
> };
> + unsigned long nr_dirty = read_page_state(nr_dirty);
> + unsigned long nr_unstable = read_page_state(nr_unstable);
read_page_state doesn't exist in 2.6.7-rc3 or 2.6.6-mm5. How is it
defined?
If it is just a simple macro to access the right fields, then the
patch looks fine to me.
> - get_page_state(&ps);
> - wbc.nr_to_write = ps.nr_dirty + ps.nr_unstable +
> + wbc.nr_to_write = nr_dirty + nr_unstable +
> (inodes_stat.nr_inodes - inodes_stat.nr_unused) +
> - ps.nr_dirty + ps.nr_unstable;
> + nr_dirty + nr_unstable;
> wbc.nr_to_write += wbc.nr_to_write / 2; /* Bit more for luck */
> spin_lock(&inode_lock);
> sync_sb_inodes(sb, &wbc);
> _
Jörn
--
Fantasy is more important than knowledge. Knowledge is limited,
while fantasy embraces the whole world.
-- Albert Einstein
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-10 22:59 ` Jörn Engel
@ 2004-06-10 23:10 ` Andrew Morton
2004-06-11 10:53 ` Jörn Engel
2004-10-15 1:58 ` Rusty Russell
0 siblings, 2 replies; 21+ messages in thread
From: Andrew Morton @ 2004-06-10 23:10 UTC (permalink / raw)
To: Jörn Engel; +Cc: michael, linux-kernel, hugh
Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:
>
> read_page_state doesn't exist in 2.6.7-rc3 or 2.6.6-mm5. How is it
> defined?
It was in 2.6.7-rc3-mm1.
struct page_state is large (148 bytes) and we put them on the stack in awkward
code paths (page reclaim...)
So implement a simple read_page_state() which can be used to pluck out a
single member from the all-cpus page_state accumulators.
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/include/linux/page-flags.h | 4 ++++
25-akpm/mm/page_alloc.c | 17 +++++++++++++++++
2 files changed, 21 insertions(+)
diff -puN include/linux/page-flags.h~read_page_state include/linux/page-flags.h
--- 25/include/linux/page-flags.h~read_page_state Wed Jun 9 17:06:33 2004
+++ 25-akpm/include/linux/page-flags.h Wed Jun 9 17:06:33 2004
@@ -139,6 +139,10 @@ DECLARE_PER_CPU(struct page_state, page_
extern void get_page_state(struct page_state *ret);
extern void get_full_page_state(struct page_state *ret);
+extern unsigned long __read_page_state(unsigned offset);
+
+#define read_page_state(member) \
+ __read_page_state(offsetof(struct page_state, member))
#define mod_page_state(member, delta) \
do { \
diff -puN mm/page_alloc.c~read_page_state mm/page_alloc.c
--- 25/mm/page_alloc.c~read_page_state Wed Jun 9 17:06:33 2004
+++ 25-akpm/mm/page_alloc.c Wed Jun 9 17:11:57 2004
@@ -947,6 +947,23 @@ void get_full_page_state(struct page_sta
__get_page_state(ret, sizeof(*ret) / sizeof(unsigned long));
}
+unsigned long __read_page_state(unsigned offset)
+{
+ unsigned long ret = 0;
+ int cpu;
+
+ for (cpu = 0; cpu < NR_CPUS; cpu++) {
+ unsigned long in;
+
+ if (!cpu_possible(cpu))
+ continue;
+
+ in = (unsigned long)&per_cpu(page_states, cpu) + offset;
+ ret += *((unsigned long *)in);
+ }
+ return ret;
+}
+
void get_zone_counts(unsigned long *active,
unsigned long *inactive, unsigned long *free)
{
_
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-10 23:10 ` Andrew Morton
@ 2004-06-11 10:53 ` Jörn Engel
2004-10-15 1:58 ` Rusty Russell
1 sibling, 0 replies; 21+ messages in thread
From: Jörn Engel @ 2004-06-11 10:53 UTC (permalink / raw)
To: Andrew Morton; +Cc: michael, linux-kernel, hugh
On Thu, 10 June 2004 16:10:21 -0700, Andrew Morton wrote:
> Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:
> >
> > read_page_state doesn't exist in 2.6.7-rc3 or 2.6.6-mm5. How is it
> > defined?
>
> It was in 2.6.7-rc3-mm1.
A beauty! Your trick about submitting an ugly patch and waiting for
others to do something better really works. :)
Jörn
--
My second remark is that our intellectual powers are rather geared to
master static relations and that our powers to visualize processes
evolving in time are relatively poorly developed.
-- Edsger W. Dijkstra
^ permalink raw reply [flat|nested] 21+ 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-09 15:23 ` Michael Clark
@ 2004-06-15 23:34 ` Randy.Dunlap
2004-06-16 7:11 ` Florian Schirmer
1 sibling, 1 reply; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ 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; 21+ 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] 21+ messages in thread
* Re: [STACK] >3k call path in ide
2004-06-10 23:10 ` Andrew Morton
2004-06-11 10:53 ` Jörn Engel
@ 2004-10-15 1:58 ` Rusty Russell
1 sibling, 0 replies; 21+ messages in thread
From: Rusty Russell @ 2004-10-15 1:58 UTC (permalink / raw)
To: Andrew Morton; +Cc: Jörn Engel, michael, lkml - Kernel Mailing List, hugh
On Fri, 2004-06-11 at 09:10, Andrew Morton wrote:
> Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:
> >
> > read_page_state doesn't exist in 2.6.7-rc3 or 2.6.6-mm5. How is it
> > defined?
>
> It was in 2.6.7-rc3-mm1.
>
>
>
> struct page_state is large (148 bytes) and we put them on the stack in awkward
> code paths (page reclaim...)
>
> So implement a simple read_page_state() which can be used to pluck out a
> single member from the all-cpus page_state accumulators.
>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
...
> +unsigned long __read_page_state(unsigned offset)
> +{
> + unsigned long ret = 0;
> + int cpu;
> +
> + for (cpu = 0; cpu < NR_CPUS; cpu++) {
> + unsigned long in;
> +
> + if (!cpu_possible(cpu))
> + continue;
for_each_cpu(cpu) here perhaps?
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2004-10-15 1:58 UTC | newest]
Thread overview: 21+ 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-09 15:23 ` Michael Clark
2004-06-09 16:29 ` Jörn Engel
2004-06-09 19:27 ` Andrew Morton
2004-06-10 22:59 ` Jörn Engel
2004-06-10 23:10 ` Andrew Morton
2004-06-11 10:53 ` Jörn Engel
2004-10-15 1:58 ` Rusty Russell
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