[RFC][PATCH] make reiserfs stop using 'struct file' for internal xattr operations

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Christoph Hellwig <hch@...>
Cc: Andrew Morton <akpm@...>, Laurent Riffard <laurent.riffard@...>, <linux-kernel@...>, <linux-fsdevel@...>, <reiserfs-devel@...>
Date: Thursday, September 27, 2007 - 4:53 pm

On Thu, 2007-09-27 at 21:26 +0100, Christoph Hellwig wrote:

I'm perplexed as to why a 'struct file' is needed, too.  The 'struct
file' doesn't get used for anything _but_ the dentry, and the functions
to which it is passed (reiserfs_prepate/commit_write()) don't use it.
In fact, some other callers just pass NULL.

This is a completely naive implementation, and I've only tested that it
compiles, but does anybody see a reason we can't just do this?

BTW, do reiserfs developers know that you can put function declarations
in header files?  ;)  4 different .c files have this:

int reiserfs_commit_write(struct page *page, unsigned from, unsigned to);
int reiserfs_prepare_write(struct page *page, unsigned from, unsigned to);

---

 lxc-dave/fs/reiserfs/inode.c |   15 ++++------
 lxc-dave/fs/reiserfs/ioctl.c |   10 ++-----
 lxc-dave/fs/reiserfs/xattr.c |   59 +++++++++++++------------------------------
 3 files changed, 28 insertions(+), 56 deletions(-)

diff -puN fs/open.c~fix-reiserfs-oops fs/open.c
diff -puN fs/file_table.c~fix-reiserfs-oops fs/file_table.c
diff -puN include/linux/mount.h~fix-reiserfs-oops include/linux/mount.h
diff -puN fs/reiserfs/xattr.c~fix-reiserfs-oops fs/reiserfs/xattr.c
--- lxc/fs/reiserfs/xattr.c~fix-reiserfs-oops	2007-09-27 13:36:38.000000000 -0700
+++ lxc-dave/fs/reiserfs/xattr.c	2007-09-27 13:49:02.000000000 -0700
@@ -194,27 +194,6 @@ static struct dentry *get_xa_file_dentry
 	return xafile;
 }
 
-/* Opens a file pointer to the attribute associated with inode */
-static struct file *open_xa_file(const struct inode *inode, const char *name,
-				 int flags)
-{
-	struct dentry *xafile;
-	struct file *fp;
-
-	xafile = get_xa_file_dentry(inode, name, flags);
-	if (IS_ERR(xafile))
-		return ERR_PTR(PTR_ERR(xafile));
-	else if (!xafile->d_inode) {
-		dput(xafile);
-		return ERR_PTR(-ENODATA);
-	}
-
-	fp = dentry_open(xafile, NULL, O_RDWR);
-	/* dentry_open dputs the dentry if it fails */
-
-	return fp;
-}
-
 /*
  * this is very similar to fs/reiserfs/dir.c:reiserfs_readdir, but
  * we need to drop the path before calling the filldir struct.  That
@@ -426,10 +405,8 @@ static inline __u32 xattr_hash(const cha
 	return csum_partial(msg, len, 0);
 }
 
-int reiserfs_commit_write(struct file *f, struct page *page,
-			  unsigned from, unsigned to);
-int reiserfs_prepare_write(struct file *f, struct page *page,
-			   unsigned from, unsigned to);
+int reiserfs_commit_write(struct page *page, unsigned from, unsigned to);
+int reiserfs_prepare_write(struct page *page, unsigned from, unsigned to);
 
 
 /* Generic extended attribute operations that can be used by xa plugins */
@@ -442,7 +419,7 @@ reiserfs_xattr_set(struct inode *inode, 
 		   size_t buffer_size, int flags)
 {
 	int err = 0;
-	struct file *fp;
+	struct dentry *dentry;
 	struct page *page;
 	char *data;
 	struct address_space *mapping;
@@ -460,18 +437,18 @@ reiserfs_xattr_set(struct inode *inode, 
 		xahash = xattr_hash(buffer, buffer_size);
 
       open_file:
-	fp = open_xa_file(inode, name, flags);
-	if (IS_ERR(fp)) {
-		err = PTR_ERR(fp);
+	dentry = get_xa_file_dentry(inode, name, flags);
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
 		goto out;
 	}
 
-	xinode = fp->f_path.dentry->d_inode;
+	xinode = dentry->d_inode;
 	REISERFS_I(inode)->i_flags |= i_has_xattr_dir;
 
 	/* we need to copy it off.. */
 	if (xinode->i_nlink > 1) {
-		fput(fp);
+		dput(dentry);
 		err = reiserfs_xattr_del(inode, name);
 		if (err < 0)
 			goto out;
@@ -485,7 +462,7 @@ reiserfs_xattr_set(struct inode *inode, 
 	newattrs.ia_size = buffer_size;
 	newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
 	mutex_lock(&xinode->i_mutex);
-	err = notify_change(fp->f_path.dentry, &newattrs);
+	err = notify_change(dentry, &newattrs);
 	if (err)
 		goto out_filp;
 
@@ -518,13 +495,13 @@ reiserfs_xattr_set(struct inode *inode, 
 			rxh->h_hash = cpu_to_le32(xahash);
 		}
 
-		err = reiserfs_prepare_write(fp, page, page_offset,
+		err = reiserfs_prepare_write(page, page_offset,
 					    page_offset + chunk + skip);
 		if (!err) {
 			if (buffer)
 				memcpy(data + skip, buffer + buffer_pos, chunk);
 			err =
-			    reiserfs_commit_write(fp, page, page_offset,
+			    reiserfs_commit_write(page, page_offset,
 						  page_offset + chunk +
 						  skip);
 		}
@@ -548,7 +525,7 @@ reiserfs_xattr_set(struct inode *inode, 
 
       out_filp:
 	mutex_unlock(&xinode->i_mutex);
-	fput(fp);
+	dput(dentry);
 
       out:
 	return err;
@@ -562,7 +539,7 @@ reiserfs_xattr_get(const struct inode *i
 		   size_t buffer_size)
 {
 	ssize_t err = 0;
-	struct file *fp;
+	struct dentry *dentry;
 	size_t isize;
 	size_t file_pos = 0;
 	size_t buffer_pos = 0;
@@ -578,13 +555,13 @@ reiserfs_xattr_get(const struct inode *i
 	if (get_inode_sd_version(inode) == STAT_DATA_V1)
 		return -EOPNOTSUPP;
 
-	fp = open_xa_file(inode, name, FL_READONLY);
-	if (IS_ERR(fp)) {
-		err = PTR_ERR(fp);
+	dentry = get_xa_file_dentry(inode, name, FL_READONLY);
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
 		goto out;
 	}
 
-	xinode = fp->f_path.dentry->d_inode;
+	xinode = dentry->d_inode;
 	isize = xinode->i_size;
 	REISERFS_I(inode)->i_flags |= i_has_xattr_dir;
 
@@ -652,7 +629,7 @@ reiserfs_xattr_get(const struct inode *i
 	}
 
       out_dput:
-	fput(fp);
+	dput(dentry);
 
       out:
 	return err;
diff -puN fs/reiserfs/inode.c~fix-reiserfs-oops fs/reiserfs/inode.c
--- lxc/fs/reiserfs/inode.c~fix-reiserfs-oops	2007-09-27 13:39:47.000000000 -0700
+++ lxc-dave/fs/reiserfs/inode.c	2007-09-27 13:45:03.000000000 -0700
@@ -19,10 +19,8 @@
 #include <linux/quotaops.h>
 #include <linux/swap.h>
 
-int reiserfs_commit_write(struct file *f, struct page *page,
-			  unsigned from, unsigned to);
-int reiserfs_prepare_write(struct file *f, struct page *page,
-			   unsigned from, unsigned to);
+int reiserfs_commit_write(struct page *page,  unsigned from, unsigned to);
+int reiserfs_prepare_write(struct page *page, unsigned from, unsigned to);
 
 void reiserfs_delete_inode(struct inode *inode)
 {
@@ -550,14 +548,14 @@ static int convert_tail_for_hole(struct 
 	 ** won't trigger a get_block in this case.
 	 */
 	fix_tail_page_for_writing(tail_page);
-	retval = reiserfs_prepare_write(NULL, tail_page, tail_start, tail_end);
+	retval = reiserfs_prepare_write(tail_page, tail_start, tail_end);
 	if (retval)
 		goto unlock;
 
 	/* tail conversion might change the data in the page */
 	flush_dcache_page(tail_page);
 
-	retval = reiserfs_commit_write(NULL, tail_page, tail_start, tail_end);
+	retval = reiserfs_commit_write(tail_page, tail_start, tail_end);
 
       unlock:
 	if (tail_page != hole_page) {
@@ -2613,8 +2611,7 @@ static int reiserfs_write_begin(struct f
 	return ret;
 }
 
-int reiserfs_prepare_write(struct file *f, struct page *page,
-			   unsigned from, unsigned to)
+int reiserfs_prepare_write(struct page *page, unsigned from, unsigned to)
 {
 	struct inode *inode = page->mapping->host;
 	int ret;
@@ -2761,7 +2758,7 @@ static int reiserfs_write_end(struct fil
 	goto out;
 }
 
-int reiserfs_commit_write(struct file *f, struct page *page,
+int reiserfs_commit_write(struct page *page,
 			  unsigned from, unsigned to)
 {
 	struct inode *inode = page->mapping->host;
diff -puN fs/reiserfs/ioctl.c~fix-reiserfs-oops fs/reiserfs/ioctl.c
--- lxc/fs/reiserfs/ioctl.c~fix-reiserfs-oops	2007-09-27 13:42:07.000000000 -0700
+++ lxc-dave/fs/reiserfs/ioctl.c	2007-09-27 13:45:17.000000000 -0700
@@ -143,10 +143,8 @@ long reiserfs_compat_ioctl(struct file *
 }
 #endif
 
-int reiserfs_commit_write(struct file *f, struct page *page,
-			  unsigned from, unsigned to);
-int reiserfs_prepare_write(struct file *f, struct page *page,
-			   unsigned from, unsigned to);
+int reiserfs_commit_write(struct page *page, unsigned from, unsigned to);
+int reiserfs_prepare_write(struct page *page, unsigned from, unsigned to);
 /*
 ** reiserfs_unpack
 ** Function try to convert tail from direct item into indirect.
@@ -194,13 +192,13 @@ static int reiserfs_unpack(struct inode 
 	if (!page) {
 		goto out;
 	}
-	retval = reiserfs_prepare_write(NULL, page, write_from, write_from);
+	retval = reiserfs_prepare_write(page, write_from, write_from);
 	if (retval)
 		goto out_unlock;
 
 	/* conversion can change page contents, must flush */
 	flush_dcache_page(page);
-	retval = reiserfs_commit_write(NULL, page, write_from, write_from);
+	retval = reiserfs_commit_write(page, write_from, write_from);
 	REISERFS_I(inode)->i_flags |= i_nopack_mask;
 
       out_unlock:
_


-- Dave

-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
2.6.23-rc8-mm2, Andrew Morton, (Thu Sep 27, 5:22 am)
Re: 2.6.23-rc8-mm2, Matt Mackall, (Tue Oct 9, 1:49 pm)
Re: 2.6.23-rc8-mm2 - PowerPC link failure at arch/powerpc/ke..., Kamalesh Babulal, (Sun Sep 30, 12:10 am)
Re: 2.6.23-rc8-mm2, , (Sat Sep 29, 10:26 pm)
Re: 2.6.23-rc8-mm2, Andrew Morton, (Sun Sep 30, 4:50 am)
Re: 2.6.23-rc8-mm2, , (Mon Oct 1, 12:12 pm)
Re: 2.6.23-rc8-mm2, Rafael J. Wysocki, (Sun Sep 30, 4:01 pm)
Re: 2.6.23-rc8-mm2, , (Mon Oct 1, 1:12 pm)
Re: 2.6.23-rc8-mm2, Dave Young, (Sat Sep 29, 5:37 am)
Re: 2.6.23-rc8-mm2, Greg KH, (Sat Sep 29, 11:19 am)
Re: 2.6.23-rc8-mm2, Dave Young, (Mon Oct 8, 2:43 am)
Re: 2.6.23-rc8-mm2, , (Sun Sep 30, 1:18 am)
Re: 2.6.23-rc8-mm2, Dave Young, (Sat Sep 29, 9:29 pm)
/proc/net/ bad hard links count [Was: 2.6.23-rc8-mm2], Jiri Slaby, (Fri Sep 28, 12:30 pm)
Re: /proc/net/ bad hard links count [Was: 2.6.23-rc8-mm2], Eric W. Biederman, (Fri Sep 28, 1:03 pm)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Cedric Le Goater, (Fri Sep 28, 11:42 am)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Fri Sep 28, 3:10 pm)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Sat Sep 29, 8:44 am)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Cedric Le Goater, (Sat Sep 29, 10:55 am)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Sat Sep 29, 4:49 pm)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Cedric Le Goater, (Mon Oct 1, 5:26 am)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Tue Oct 2, 6:26 am)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Tue Oct 2, 4:06 pm)
Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING, Ilpo Järvinen, (Tue Oct 2, 5:48 pm)
Re: 2.6.23-rc8-mm2: BUG near reiserfs_xattr_set, Laurent Riffard, (Thu Sep 27, 3:18 pm)
Re: 2.6.23-rc8-mm2: BUG near reiserfs_xattr_set, Andrew Morton, (Thu Sep 27, 3:48 pm)
Re: 2.6.23-rc8-mm2: BUG near reiserfs_xattr_set, Christoph Hellwig, (Thu Sep 27, 4:26 pm)
[RFC][PATCH] make reiserfs stop using 'struct file' for inte..., Dave Hansen, (Thu Sep 27, 4:53 pm)
Re: [RFC][PATCH] make reiserfs stop using 'struct file' for ..., Christoph Hellwig, (Thu Sep 27, 5:04 pm)
Re: [RFC][PATCH] make reiserfs stop using 'struct file' for ..., Christoph Hellwig, (Fri Sep 28, 3:16 am)
[RFC][PATCH] stop abusing filp_open in reiserfs journal code, Christoph Hellwig, (Fri Sep 28, 3:29 am)
Re: 2.6.23-rc8-mm2: BUG near reiserfs_xattr_set, Dave Hansen, (Thu Sep 27, 4:05 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Rafael J. Wysocki, (Thu Sep 27, 11:19 am)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Rafael J. Wysocki, (Thu Sep 27, 11:59 am)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Sam Ravnborg, (Thu Sep 27, 12:53 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Randy Dunlap, (Thu Sep 27, 1:33 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Sam Ravnborg, (Thu Sep 27, 3:19 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Rafael J. Wysocki, (Thu Sep 27, 3:48 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Randy Dunlap, (Thu Sep 27, 3:37 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Rafael J. Wysocki, (Thu Sep 27, 4:01 pm)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Thomas Gleixner, (Thu Sep 27, 11:49 am)
Re: 2.6.23-rc8-mm2: problems on HP nx6325, Rafael J. Wysocki, (Fri Sep 28, 5:31 pm)
Re: 2.6.23-rc8-mm2 - drivers/net/ibm_newemac/mal - broken, Kamalesh Babulal, (Thu Sep 27, 6:52 am)