[PATCH] reduce stack in cdrom/optcd.c

From: Randy.Dunlap (rddunlap@osdl.org)
Date: Wed Mar 26 2003 - 15:12:50 EST

(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/

This archive was generated by hypermail 2b29 : Mon Mar 31 2003 - 22:00:25 EST