From: Christoph Hellwig <hch@infradead.org>
To: npiggin@suse.de
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, Christoph Hellwig <hch@lst.de>
Subject: [PATCH 12/n] kill spurious reference to vmtruncate
Date: Tue, 22 Sep 2009 16:53:18 -0400 [thread overview]
Message-ID: <20090922205318.GA12224@infradead.org> (raw)
In-Reply-To: <20090820163504.131529718@suse.de>
Lots of filesystems calls vmtruncate despite not implementing the old
->truncate method. Switch them to use simple_setsize and add some
comments about the truncate code where it seems fitting.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: vfs-2.6.git/fs/ufs/truncate.c
===================================================================
--- vfs-2.6.git.orig/fs/ufs/truncate.c 2009-09-22 13:08:36.842264538 -0300
+++ vfs-2.6.git/fs/ufs/truncate.c 2009-09-22 13:12:13.926305949 -0300
@@ -500,12 +500,10 @@ out:
return err;
}
-
/*
- * We don't define our `inode->i_op->truncate', and call it here,
- * because of:
- * - there is no way to know old size
- * - there is no way inform user about error, if it happens in `truncate'
+ * TODO:
+ * - truncate case should use proper ordering instead of using
+ * simple_setsize
*/
static int ufs_setattr(struct dentry *dentry, struct iattr *attr)
{
@@ -520,7 +518,7 @@ static int ufs_setattr(struct dentry *de
if (ia_valid & ATTR_SIZE &&
attr->ia_size != i_size_read(inode)) {
loff_t old_i_size = inode->i_size;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
return error;
error = ufs_truncate(inode, old_i_size);
Index: vfs-2.6.git/fs/adfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/adfs/inode.c 2009-09-22 13:18:00.242265034 -0300
+++ vfs-2.6.git/fs/adfs/inode.c 2009-09-22 13:18:33.437781151 -0300
@@ -328,8 +328,9 @@ adfs_notify_change(struct dentry *dentry
if (error)
goto out;
+ /* XXX: this is missing some actual on-disk truncation.. */
if (ia_valid & ATTR_SIZE)
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
goto out;
Index: vfs-2.6.git/fs/ecryptfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/ecryptfs/inode.c 2009-09-22 13:20:16.726261535 -0300
+++ vfs-2.6.git/fs/ecryptfs/inode.c 2009-09-22 13:21:30.893762356 -0300
@@ -831,9 +831,15 @@ int ecryptfs_truncate(struct dentry *den
- (new_length & ~PAGE_CACHE_MASK));
if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
- rc = vmtruncate(inode, new_length);
+ rc = simple_setsize(inode, new_length);
if (rc)
goto out_free;
+
+ /*
+ * XXX: this is horribly buggy as we can't and never
+ * could rely on the lower inode having a ->truncate
+ * method to actually release blocks.
+ */
rc = vmtruncate(lower_dentry->d_inode, new_length);
goto out_free;
}
@@ -855,7 +861,7 @@ int ecryptfs_truncate(struct dentry *den
goto out_free;
}
}
- vmtruncate(inode, new_length);
+ simple_setsize(inode, new_length);
rc = ecryptfs_write_inode_size_to_metadata(inode);
if (rc) {
printk(KERN_ERR "Problem with "
@@ -870,6 +876,11 @@ int ecryptfs_truncate(struct dentry *den
lower_size_after_truncate =
upper_size_to_lower_size(crypt_stat, new_length);
if (lower_size_after_truncate < lower_size_before_truncate)
+ /*
+ * XXX: this is horribly buggy as we can't and never
+ * could rely on the lower inode having a ->truncate
+ * method to actually release blocks.
+ */
vmtruncate(lower_dentry->d_inode,
lower_size_after_truncate);
}
Index: vfs-2.6.git/fs/gfs2/aops.c
===================================================================
--- vfs-2.6.git.orig/fs/gfs2/aops.c 2009-09-22 13:15:40.718262195 -0300
+++ vfs-2.6.git/fs/gfs2/aops.c 2009-09-22 13:16:45.265763898 -0300
@@ -710,8 +710,14 @@ out:
return 0;
page_cache_release(page);
+
+ /*
+ * XXX(hch): the call below should probably be replaced with
+ * a call to the gfs2-specific truncate blocks helper to actually
+ * release disk blocks..
+ */
if (pos + len > ip->i_inode.i_size)
- vmtruncate(&ip->i_inode, ip->i_inode.i_size);
+ simple_setsize(&ip->i_inode, ip->i_inode.i_size);
out_endtrans:
gfs2_trans_end(sdp);
out_trans_fail:
Index: vfs-2.6.git/fs/gfs2/ops_inode.c
===================================================================
--- vfs-2.6.git.orig/fs/gfs2/ops_inode.c 2009-09-22 13:16:48.901764352 -0300
+++ vfs-2.6.git/fs/gfs2/ops_inode.c 2009-09-22 13:17:30.256367942 -0300
@@ -1129,6 +1129,9 @@ int gfs2_permission(struct inode *inode,
return error;
}
+/*
+ * XXX: should be changed to have proper ordering by opencoding simple_setsize
+ */
static int setattr_size(struct inode *inode, struct iattr *attr)
{
struct gfs2_inode *ip = GFS2_I(inode);
@@ -1139,7 +1142,7 @@ static int setattr_size(struct inode *in
error = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
if (error)
return error;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
gfs2_trans_end(sdp);
if (error)
return error;
Index: vfs-2.6.git/fs/jffs2/fs.c
===================================================================
--- vfs-2.6.git.orig/fs/jffs2/fs.c 2009-09-22 13:18:47.962264848 -0300
+++ vfs-2.6.git/fs/jffs2/fs.c 2009-09-22 13:21:42.981761532 -0300
@@ -169,13 +169,13 @@ int jffs2_do_setattr (struct inode *inod
mutex_unlock(&f->sem);
jffs2_complete_reservation(c);
- /* We have to do the vmtruncate() without f->sem held, since
+ /* We have to do the simple_setsize() without f->sem held, since
some pages may be locked and waiting for it in readpage().
We are protected from a simultaneous write() extending i_size
back past iattr->ia_size, because do_truncate() holds the
generic inode semaphore. */
if (ivalid & ATTR_SIZE && inode->i_size > iattr->ia_size) {
- vmtruncate(inode, iattr->ia_size);
+ simple_setsize(inode, iattr->ia_size);
inode->i_blocks = (inode->i_size + 511) >> 9;
}
Index: vfs-2.6.git/fs/ocfs2/file.c
===================================================================
--- vfs-2.6.git.orig/fs/ocfs2/file.c 2009-09-22 13:14:22.849791263 -0300
+++ vfs-2.6.git/fs/ocfs2/file.c 2009-09-22 13:15:26.746262469 -0300
@@ -1021,7 +1021,7 @@ int ocfs2_setattr(struct dentry *dentry,
}
/*
- * This will intentionally not wind up calling vmtruncate(),
+ * This will intentionally not wind up calling simple_setsize(),
* since all the work for a size change has been done above.
* Otherwise, we could get into problems with truncate as
* ip_alloc_sem is used there to protect against i_size
@@ -1864,9 +1864,13 @@ relock:
* direct write may have instantiated a few
* blocks outside i_size. Trim these off again.
* Don't need i_size_read because we hold i_mutex.
+ *
+ * XXX(hch): this looks buggy because ocfs2 did not
+ * actually implement ->truncate. Take a look at
+ * the new truncate sequence and update this accordingly
*/
if (*ppos + count > inode->i_size)
- vmtruncate(inode, inode->i_size);
+ simple_setsize(inode, inode->i_size);
ret = written;
goto out_dio;
}
Index: vfs-2.6.git/fs/smbfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/smbfs/inode.c 2009-09-22 13:12:18.913764201 -0300
+++ vfs-2.6.git/fs/smbfs/inode.c 2009-09-22 13:12:48.649797002 -0300
@@ -712,7 +712,7 @@ smb_notify_change(struct dentry *dentry,
error = server->ops->truncate(inode, attr->ia_size);
if (error)
goto out;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
goto out;
refresh = 1;
Index: vfs-2.6.git/fs/ubifs/file.c
===================================================================
--- vfs-2.6.git.orig/fs/ubifs/file.c 2009-09-22 13:12:56.969768178 -0300
+++ vfs-2.6.git/fs/ubifs/file.c 2009-09-22 13:13:59.161870394 -0300
@@ -966,12 +966,15 @@ static int do_writepage(struct page *pag
* the page locked, and it locks @ui_mutex. However, write-back does take inode
* @i_mutex, which means other VFS operations may be run on this inode at the
* same time. And the problematic one is truncation to smaller size, from where
- * we have to call 'vmtruncate()', which first changes @inode->i_size, then
+ * we have to call 'simple_setsize()', which first changes @inode->i_size, then
* drops the truncated pages. And while dropping the pages, it takes the page
- * lock. This means that 'do_truncation()' cannot call 'vmtruncate()' with
+ * lock. This means that 'do_truncation()' cannot call 'simple_setsize()' with
* @ui_mutex locked, because it would deadlock with 'ubifs_writepage()'. This
* means that @inode->i_size is changed while @ui_mutex is unlocked.
*
+ * XXX: with the new truncate the above is not true anymore, the simple_setsize
+ * calls can be replaced with the individual components.
+ *
* But in 'ubifs_writepage()' we have to guarantee that we do not write beyond
* inode size. How do we do this if @inode->i_size may became smaller while we
* are in the middle of 'ubifs_writepage()'? The UBIFS solution is the
@@ -1124,7 +1127,7 @@ static int do_truncation(struct ubifs_in
budgeted = 0;
}
- err = vmtruncate(inode, new_size);
+ err = simple_setsize(inode, new_size);
if (err)
goto out_budg;
@@ -1213,7 +1216,7 @@ static int do_setattr(struct ubifs_info
if (attr->ia_valid & ATTR_SIZE) {
dbg_gen("size %lld -> %lld", inode->i_size, new_size);
- err = vmtruncate(inode, new_size);
+ err = simple_setsize(inode, new_size);
if (err)
goto out;
}
@@ -1222,7 +1225,7 @@ static int do_setattr(struct ubifs_info
if (attr->ia_valid & ATTR_SIZE) {
/* Truncation changes inode [mc]time */
inode->i_mtime = inode->i_ctime = ubifs_current_time(inode);
- /* 'vmtruncate()' changed @i_size, update @ui_size */
+ /* 'simple_setsize()' changed @i_size, update @ui_size */
ui->ui_size = inode->i_size;
}
Index: vfs-2.6.git/fs/ubifs/ubifs.h
===================================================================
--- vfs-2.6.git.orig/fs/ubifs/ubifs.h 2009-09-22 13:14:03.945800885 -0300
+++ vfs-2.6.git/fs/ubifs/ubifs.h 2009-09-22 13:14:13.014300584 -0300
@@ -378,7 +378,7 @@ struct ubifs_gced_idx_leb {
* The @ui_size is a "shadow" variable for @inode->i_size and UBIFS uses
* @ui_size instead of @inode->i_size. The reason for this is that UBIFS cannot
* make sure @inode->i_size is always changed under @ui_mutex, because it
- * cannot call 'vmtruncate()' with @ui_mutex locked, because it would deadlock
+ * cannot call 'simple_setsize()' with @ui_mutex locked, because it would deadlock
* with 'ubifs_writepage()' (see file.c). All the other inode fields are
* changed under @ui_mutex, so they do not need "shadow" fields. Note, one
* could consider to rework locking and base it on "shadow" fields.
next prev parent reply other threads:[~2009-09-22 20:53 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
2009-08-26 7:38 ` Artem Bityutskiy
2009-09-07 7:33 ` Nick Piggin
2009-09-07 7:48 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 02/11] fs: use " npiggin
2009-08-20 16:35 ` npiggin-l3A5Bk7waGM
2009-08-20 16:35 ` [patch 03/11] fs: introduce new truncate sequence npiggin
2009-08-26 7:40 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 04/11] fs: convert simple fs to new truncate npiggin
2009-08-20 16:35 ` [patch 05/11] tmpfs: convert to use the new truncate convention npiggin
2009-08-20 16:35 ` [patch 06/11] ext2: " npiggin
2009-08-21 13:42 ` Jan Kara
2009-08-21 14:06 ` Jan Kara
2009-08-24 5:30 ` [patch] ext2: convert to use the new truncate convention fix Nick Piggin
2009-08-20 16:35 ` [patch 07/11] fat: convert to use the new truncate convention npiggin
2009-08-20 16:35 ` [patch 08/11] btrfs: " npiggin
2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 09/11] jfs: " npiggin
2009-08-20 16:35 ` [patch 10/11] udf: " npiggin
2009-08-21 14:22 ` Jan Kara
2009-08-24 5:33 ` Nick Piggin
2009-08-20 16:35 ` [patch 11/11] minix: " npiggin
2009-09-09 7:11 ` [patch 00/11] new truncate sequence Artem Bityutskiy
2009-09-22 15:04 ` Al Viro
2009-09-22 20:00 ` Christoph Hellwig
2009-09-22 21:51 ` Al Viro
2009-09-22 23:27 ` Al Viro
2009-09-22 23:58 ` Al Viro
2009-09-23 2:29 ` Al Viro
2009-09-27 19:50 ` Nick Piggin
2009-12-07 12:49 ` Nick Piggin
2009-12-07 22:46 ` Tyler Hicks
2009-09-22 20:53 ` Christoph Hellwig [this message]
2009-09-22 20:54 ` [PATCH 13/n] xfs: " Christoph Hellwig
2009-09-22 20:54 ` Christoph Hellwig
2009-09-22 20:55 ` [PATCH 14/n] sysv: " Christoph Hellwig
2009-09-22 20:56 ` [PATCH 15/n] ntfs: " Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090922205318.GA12224@infradead.org \
--to=hch@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=npiggin@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.