[patch] reiser4: do not allocate struct file on stack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <[email protected]>
---
 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);

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux