lkml.org 
[lkml]   [2007]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch] reiser4: do not allocate struct file on stack
Edward Shishkin wrote:

> Dave Hansen wrote:
>
...

>>
>> I think that stack allocation is a pretty nasty trick for a structure
>> that's supposed to be pretty persistent and dynamically allocated, and
>> is certainly something that needs to get fixed up in a proper way.
>>
>>
>
> agreed.
>
>> This works around the problem for now, but this could potentially cause
>> more bugs any time we add some member to 'struct file' and depend on its
>> state being sane anywhere in the VFS. If there's a list anywhere of
>> merge-stopper reiser4 bugs around, this should probably go in there.
>>
>>
>
> will be fixed.
>

The promised fixup is attached.
Andrew, please apply.

Thanks,
Edward.
Do not allocate struct file on stack, pass the persistent one instead.

Signed-off-by: Edward Shishkin <edward@namesys.com>
---
linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c | 35 ++++------
linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h | 2
linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c | 23 ++----
3 files changed, 26 insertions(+), 34 deletions(-)
--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c
@@ -566,23 +566,18 @@
* items or add them to represent a hole at the end of file. The caller has to
* obtain exclusive access to the file.
*/
-static int truncate_file_body(struct inode *inode, loff_t new_size)
+static int truncate_file_body(struct inode *inode, struct iattr *attr)
{
int result;
+ loff_t new_size = attr->ia_size;

if (inode->i_size < new_size) {
/* expanding truncate */
- struct dentry dentry;
- struct file file;
- struct unix_file_info *uf_info;
+ struct file * file = attr->ia_file;
+ struct unix_file_info *uf_info = unix_file_inode_data(inode);
+
+ assert("edward-1532", attr->ia_valid & ATTR_FILE);

- dentry.d_inode = inode;
- file.f_dentry = &dentry;
- file.private_data = NULL;
- file.f_pos = new_size;
- file.private_data = NULL;
- file.f_vfsmnt = NULL;
- uf_info = unix_file_inode_data(inode);
result = find_file_state(inode, uf_info);
if (result)
return result;
@@ -615,19 +610,19 @@
return result;
}
}
- result = reiser4_write_extent(&file, NULL, 0,
+ result = reiser4_write_extent(file, NULL, 0,
&new_size);
if (result)
return result;
uf_info->container = UF_CONTAINER_EXTENTS;
} else {
if (uf_info->container == UF_CONTAINER_EXTENTS) {
- result = reiser4_write_extent(&file, NULL, 0,
+ result = reiser4_write_extent(file, NULL, 0,
&new_size);
if (result)
return result;
} else {
- result = reiser4_write_tail(&file, NULL, 0,
+ result = reiser4_write_tail(file, NULL, 0,
&new_size);
if (result)
return result;
@@ -636,10 +631,10 @@
}
BUG_ON(result > 0);
INODE_SET_FIELD(inode, i_size, new_size);
- file_update_time(&file);
+ file_update_time(file);
result = reiser4_update_sd(inode);
BUG_ON(result != 0);
- reiser4_free_file_fsdata(&file);
+ reiser4_free_file_fsdata(file);
} else
result = shorten_file(inode, new_size);
return result;
@@ -2092,7 +2087,7 @@
* first item is formatting item, therefore there was
* incomplete extent2tail conversion. Complete it
*/
- result = extent2tail(unix_file_inode_data(inode));
+ result = extent2tail(file, unix_file_inode_data(inode));
else
result = -EIO;

@@ -2372,7 +2367,7 @@
uf_info->container == UF_CONTAINER_EXTENTS &&
!should_have_notail(uf_info, inode->i_size) &&
!rofs_inode(inode)) {
- result = extent2tail(uf_info);
+ result = extent2tail(file, uf_info);
if (result != 0) {
warning("nikita-3233",
"Failed (%d) to convert in %s (%llu)",
@@ -2638,7 +2633,7 @@
if (result == 0)
result = safe_link_add(inode, SAFE_TRUNCATE);
if (result == 0)
- result = truncate_file_body(inode, attr->ia_size);
+ result = truncate_file_body(inode, attr);
if (result)
warning("vs-1588", "truncate_file failed: oid %lli, "
"old size %lld, new size %lld, retval %d",
@@ -2724,7 +2719,7 @@
/* truncate file bogy first */
uf_info = unix_file_inode_data(inode);
get_exclusive_access(uf_info);
- result = truncate_file_body(inode, 0 /* size */ );
+ result = shorten_file(inode, 0 /* size */ );
drop_exclusive_access(uf_info);

if (result)
--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h
@@ -237,7 +237,7 @@
#define WRITE_GRANULARITY 32

int tail2extent(struct unix_file_info *);
-int extent2tail(struct unix_file_info *);
+int extent2tail(struct file *, struct unix_file_info *);

int goto_right_neighbor(coord_t *, lock_handle *);
int find_or_create_extent(struct page *);
--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c
@@ -546,7 +546,7 @@

/* for every page of file: read page, cut part of extent pointing to this page,
put data of page tree by tail item */
-int extent2tail(struct unix_file_info *uf_info)
+int extent2tail(struct file * file, struct unix_file_info *uf_info)
{
int result;
struct inode *inode;
@@ -644,20 +644,17 @@
/* last page can be incompleted */
count = (inode->i_size & ~PAGE_CACHE_MASK);
while (count) {
- struct dentry dentry;
- struct file file;
- loff_t pos;
-
- dentry.d_inode = inode;
- file.f_dentry = &dentry;
- file.private_data = NULL;
- file.f_pos = start_byte;
- file.private_data = NULL;
- pos = start_byte;
- result = reiser4_write_tail(&file,
+ loff_t pos = start_byte;
+
+ assert("edward-1533",
+ file != NULL && file->f_dentry != NULL);
+ assert("edward-1534",
+ file->f_dentry->d_inode == inode);
+
+ result = reiser4_write_tail(file,
(char __user *)kmap(page),
count, &pos);
- reiser4_free_file_fsdata(&file);
+ reiser4_free_file_fsdata(file);
if (result <= 0) {
warning("", "reiser4_write_tail failed");
page_cache_release(page);
\
 
 \ /
  Last update: 2007-10-05 01:25    [from the cache]
©2003-2011 Jasper Spaans