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: 35+ 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-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 ` [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: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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).