Hi!
This moves two 4k buffers from stack to heap. Compiles, untested, but
looks trivial.
Alan, is this something for your tree?
J?rn
--
When people work hard for you for a pat on the back, you've got
to give them that pat.
-- Robert Heinlein
--- linux-2.5.64/fs/intermezzo/journal.c Mon Feb 24 20:05:05 2003
+++ linux-2.5.64-i2o/fs/intermezzo/journal.c Thu Mar 13 13:14:12 2003
@@ -1245,6 +1245,7 @@
struct file *f;
int len;
loff_t read_off, write_off, bytes;
+ char *buf;
ENTRY;
@@ -1255,15 +1256,18 @@
return f;
}
+ buf = kmalloc(4096, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
write_off = 0;
read_off = start;
bytes = fset->fset_kml.fd_offset - start;
while (bytes > 0) {
- char buf[4096];
int toread;
- if (bytes > sizeof(buf))
- toread = sizeof(buf);
+ if (bytes > sizeof(*buf))
+ toread = sizeof(*buf);
else
toread = bytes;
@@ -1274,6 +1278,7 @@
if (presto_fwrite(f, buf, len, &write_off) != len) {
filp_close(f, NULL);
+ kfree(buf);
EXIT;
return ERR_PTR(-EIO);
}
@@ -1281,6 +1286,7 @@
bytes -= len;
}
+ kfree(buf);
EXIT;
return f;
}
@@ -1589,7 +1595,7 @@
{
int opcode = KML_OPCODE_GET_FILEID;
struct rec_info rec;
- char *buffer, *path, *logrecord, record[4096]; /*include path*/
+ char *buffer, *path, *logrecord, *record; /*include path*/
struct dentry *root;
__u32 uid, gid, pathlen;
int error, size;
@@ -1597,6 +1603,10 @@
ENTRY;
+ record = kmalloc(4096, GFP_KERNEL);
+ if (!record)
+ return -ENOMEM;
+
root = fset->fset_dentry;
uid = cpu_to_le32(dentry->d_inode->i_uid);
@@ -1610,7 +1620,7 @@
sizeof(struct kml_suffix);
CDEBUG(D_FILE, "kml size: %d\n", size);
- if ( size > sizeof(record) )
+ if ( size > sizeof(*record) )
CERROR("InterMezzo: BUFFER OVERFLOW in %s!\n", __FUNCTION__);
memset(&rec, 0, sizeof(rec));
@@ -1633,6 +1643,7 @@
fset->fset_name);
BUFF_FREE(buffer);
+ kfree(record);
EXIT;
return error;
}
On Fri, 14 Mar 2003 16:53:52 +0100 Joern Engel <[email protected]> wrote:
| Hi!
|
| This moves two 4k buffers from stack to heap. Compiles, untested, but
| looks trivial.
| --
|
| --- linux-2.5.64/fs/intermezzo/journal.c Mon Feb 24 20:05:05 2003
| +++ linux-2.5.64-i2o/fs/intermezzo/journal.c Thu Mar 13 13:14:12 2003
| @@ -1245,6 +1245,7 @@
| struct file *f;
| int len;
| loff_t read_off, write_off, bytes;
| + char *buf;
|
| ENTRY;
|
| @@ -1255,15 +1256,18 @@
| return f;
| }
|
| + buf = kmalloc(4096, GFP_KERNEL);
| + if (!buf)
| + return ERR_PTR(-ENOMEM);
| +
| write_off = 0;
| read_off = start;
| bytes = fset->fset_kml.fd_offset - start;
| while (bytes > 0) {
| - char buf[4096];
| int toread;
|
| - if (bytes > sizeof(buf))
| - toread = sizeof(buf);
| + if (bytes > sizeof(*buf))
| + toread = sizeof(*buf);
I guess one of us needs some guidance here.
I thought that sizeof(*buf) == 1 here, not 4096. Anybody?
I don't see how sizeof() can determine the kmalloc-ed size,
so I would use BUF_SIZE instead, with
#define BUF_SIZE 4096
Same for <record> below (snipped).
--
~Randy
On Fri, 14 Mar 2003 17:44:45 +0100 Joern Engel <[email protected]> wrote:
| On Fri, 14 March 2003 08:09:30 -0800, Randy.Dunlap wrote:
| >
| > I guess one of us needs some guidance here.
| > I thought that sizeof(*buf) == 1 here, not 4096. Anybody?
| > I don't see how sizeof() can determine the kmalloc-ed size,
| > so I would use BUF_SIZE instead, with
| > #define BUF_SIZE 4096
|
| I'd love to say you're wrong, but you're not. New patch is below.
| FUNCTION_NAME_BUFSIZE should have less name collision than BUF_SIZE,
| but someone who knows intermezzo better than me might find a much
| nicer name.
If you are concerned about namespace & collisions, you can
#undef BUF_SIZE
after each function.
--
~Randy
On Fri, 14 March 2003 08:09:30 -0800, Randy.Dunlap wrote:
>
> I guess one of us needs some guidance here.
> I thought that sizeof(*buf) == 1 here, not 4096. Anybody?
> I don't see how sizeof() can determine the kmalloc-ed size,
> so I would use BUF_SIZE instead, with
> #define BUF_SIZE 4096
I'd love to say you're wrong, but you're not. New patch is below.
FUNCTION_NAME_BUFSIZE should have less name collision than BUF_SIZE,
but someone who knows intermezzo better than me might find a much
nicer name.
J?rn
--
J?rn Engel
mailto: [email protected]
http://wohnheim.fh-wedel.de/~joern
Phone: +49 179 6704074
--- linux-2.5.64/fs/intermezzo/journal.c Mon Feb 24 20:05:05 2003
+++ linux-2.5.64-i2o/fs/intermezzo/journal.c Fri Mar 14 17:37:18 2003
@@ -1239,12 +1239,15 @@
return izo_rcvd_write(fset, &rec);
}
+/* FIXME: should the below go into some header file? */
+#define PRESTO_COPY_KML_TAIL_BUFSIZE 4096
struct file * presto_copy_kml_tail(struct presto_file_set *fset,
unsigned long int start)
{
struct file *f;
int len;
loff_t read_off, write_off, bytes;
+ char *buf;
ENTRY;
@@ -1255,15 +1258,18 @@
return f;
}
+ buf = kmalloc(PRESTO_COPY_KML_TAIL_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
write_off = 0;
read_off = start;
bytes = fset->fset_kml.fd_offset - start;
while (bytes > 0) {
- char buf[4096];
int toread;
- if (bytes > sizeof(buf))
- toread = sizeof(buf);
+ if (bytes > PRESTO_COPY_KML_TAIL_BUFSIZE)
+ toread = PRESTO_COPY_KML_TAIL_BUFSIZE;
else
toread = bytes;
@@ -1274,6 +1280,7 @@
if (presto_fwrite(f, buf, len, &write_off) != len) {
filp_close(f, NULL);
+ kfree(buf);
EXIT;
return ERR_PTR(-EIO);
}
@@ -1281,6 +1288,7 @@
bytes -= len;
}
+ kfree(buf);
EXIT;
return f;
}
@@ -1584,12 +1592,14 @@
return error;
}
+/* FIXME: should the below go into some header file? */
+#define PRESTO_GET_FILEID_BUFSIZE 4096
int presto_get_fileid(int minor, struct presto_file_set *fset,
struct dentry *dentry)
{
int opcode = KML_OPCODE_GET_FILEID;
struct rec_info rec;
- char *buffer, *path, *logrecord, record[4096]; /*include path*/
+ char *buffer, *path, *logrecord, *record; /*include path*/
struct dentry *root;
__u32 uid, gid, pathlen;
int error, size;
@@ -1597,6 +1607,10 @@
ENTRY;
+ record = kmalloc(PRESTO_GET_FILEID_BUFSIZE, GFP_KERNEL);
+ if (!record)
+ return -ENOMEM;
+
root = fset->fset_dentry;
uid = cpu_to_le32(dentry->d_inode->i_uid);
@@ -1610,7 +1624,7 @@
sizeof(struct kml_suffix);
CDEBUG(D_FILE, "kml size: %d\n", size);
- if ( size > sizeof(record) )
+ if ( size > PRESTO_GET_FILEID_BUFSIZE )
CERROR("InterMezzo: BUFFER OVERFLOW in %s!\n", __FUNCTION__);
memset(&rec, 0, sizeof(rec));
@@ -1633,6 +1647,7 @@
fset->fset_name);
BUFF_FREE(buffer);
+ kfree(record);
EXIT;
return error;
}
On Fri, 14 Mar 2003 17:55:21 +0100 Joern Engel <[email protected]> wrote:
| On Fri, 14 March 2003 08:45:36 -0800, Randy.Dunlap wrote:
| >
| > If you are concerned about namespace & collisions, you can
| > #undef BUF_SIZE
| > after each function.
|
| Not, if BUF_SIZE was #defined before and should maintain that value.
| I could go and check for this specific case, but this is better left
| to the maintainer, imho.
Yes, that's right.
Actually I meant for however someone chose to spell BUF_SIZE,
but that's enough said.
Bye.
--
~Randy
On Fri, 14 March 2003 08:45:36 -0800, Randy.Dunlap wrote:
>
> If you are concerned about namespace & collisions, you can
> #undef BUF_SIZE
> after each function.
Not, if BUF_SIZE was #defined before and should maintain that value.
I could go and check for this specific case, but this is better left
to the maintainer, imho.
J?rn
--
"Error protection by error detection and correction."
-- from a university class
Go for it. Chen Yang, put it in our tree too and fix it for 2.5 as
well.
- peter -
On Fri, Mar 14, 2003 at 08:54:53AM -0800, Randy.Dunlap wrote:
> On Fri, 14 Mar 2003 17:55:21 +0100 Joern Engel <[email protected]> wrote:
>
> | On Fri, 14 March 2003 08:45:36 -0800, Randy.Dunlap wrote:
> | >
> | > If you are concerned about namespace & collisions, you can
> | > #undef BUF_SIZE
> | > after each function.
> |
> | Not, if BUF_SIZE was #defined before and should maintain that value.
> | I could go and check for this specific case, but this is better left
> | to the maintainer, imho.
>
> Yes, that's right.
>
> Actually I meant for however someone chose to spell BUF_SIZE,
> but that's enough said.
>
> Bye.
>
> --
> ~Randy
- Peter -