Messages in this thread Patch in this message | | | Date | Wed, 26 Mar 2003 12:12:50 -0800 | From | "Randy.Dunlap" <> | Subject | [PATCH] reduce stack in cdrom/optcd.c |
| |
(resend; was lost last night)
> From: Alan Cox <alan@lxorguk.ukuu.org.uk> > > On Sat, 2003-03-22 at 06:51, Randy.Dunlap wrote: > > Hi, > > > > This reduces stack usage in drivers/cdrom/optcd.c by > > dynamically allocating a large (> 2 KB) buffer. > > > > Patch is to 2.5.65. Please apply. > > This loosk broken. You are using GFP_KERNEL memory allocations on the > read path of a block device. What happens if the allocation fails > because we need memory > > Surely that buffer needs to be allocated once at open and freed on close > ? > --
Alan, Jens, anybody else-
Does this pass?
Patch is now to 2.5.66.
Thanks, ~Randy
patch_name: optcd-stackbuf.patch patch_version: 2003-03-25.19:02:22 author: Randy.Dunlap <rddunlap@osdl.org> description: pre-allocate readbuf instead of kmalloc() in cdromread(); (previously modified for stack reduction) product: Linux product_versions: 2.5.66 maintainer: unknown (email bounces) diffstat: = drivers/cdrom/optcd.c | 37 +++++++++++++++++++------------------ 1 files changed, 19 insertions(+), 18 deletions(-)
diff -Naur ./drivers/cdrom/optcd.c%OPTCD ./drivers/cdrom/optcd.c --- ./drivers/cdrom/optcd.c%OPTCD 2003-03-24 14:00:10.000000000 -0800 +++ ./drivers/cdrom/optcd.c 2003-03-25 19:01:06.000000000 -0800 @@ -1598,19 +1598,17 @@ } +static struct gendisk *optcd_disk; + + static int cdromread(unsigned long arg, int blocksize, int cmd) { - int status, ret = 0; + int status; struct cdrom_msf msf; - char *buf; if (copy_from_user(&msf, (void *) arg, sizeof msf)) return -EFAULT; - buf = kmalloc(CD_FRAMESIZE_RAWER, GFP_KERNEL); - if (!buf) - return -ENOMEM; - bin2bcd(&msf); msf.cdmsf_min1 = 0; msf.cdmsf_sec1 = 0; @@ -1619,19 +1617,15 @@ DEBUG((DEBUG_VFS, "read cmd status 0x%x", status)); - if (!sleep_flag_low(FL_DTEN, SLEEP_TIMEOUT)) { - ret = -EIO; - goto cdr_free; - } + if (!sleep_flag_low(FL_DTEN, SLEEP_TIMEOUT)) + return -EIO; - fetch_data(buf, blocksize); + fetch_data(optcd_disk->private_data, blocksize); - if (copy_to_user((void *)arg, &buf, blocksize)) - ret = -EFAULT; + if (copy_to_user((void *)arg, optcd_disk->private_data, blocksize)) + return -EFAULT; -cdr_free: - kfree(buf); - return ret; + return 0; } @@ -1857,6 +1851,14 @@ if (!open_count && state == S_IDLE) { int status; + char *buf; + + buf = kmalloc(CD_FRAMESIZE_RAWER, GFP_KERNEL); + if (!buf) { + printk(KERN_INFO "optcd: cannot allocate read buffer\n"); + return -ENOMEM; + } + optcd_disk->private_data = buf; /* save read buffer */ toc_uptodate = 0; opt_invalidate_buffers(); @@ -1922,6 +1924,7 @@ status = exec_cmd(COMOPEN); DEBUG((DEBUG_VFS, "exec_cmd COMOPEN: %02x", -status)); } + kfree(optcd_disk->private_data); del_timer(&delay_timer); del_timer(&req_timer); } @@ -2010,8 +2013,6 @@ #endif /* MODULE */ -static struct gendisk *optcd_disk; - /* Test for presence of drive and initialize it. Called at boot time or during module initialisation. */ static int __init optcd_init(void)
-- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |