On Tue, Apr 06, 2010 at 02:15:10PM +0400, Dmitry Monakhov wrote: > Al, please take a look at the patch below. > Currently it's depends on Nick's 'new truncate sequence' patch-set > Do you plan to accept Nick's patch, or i should resubmit > my patch against vfs-2.6.git/untested? I'm interested, too. -VAL > Dmitry Monakhov <dmonakhov@xxxxxxxxxx> writes: > > > This patch fix nasty bug with truncate on swapfile. > > It contains only vfs core helpers and fix isize check path for > > fs w/o i_op->setattr method. Later i'll send corresponding changes > > to other fs which has ->setattr method. > > > > This patch is depends on Nick's patch > > http://marc.info/?l=linux-fsdevel&m=126752788514574&w=2 > > > > From e8587585b2e2fefb75d47f8d50ddc7099a50873c Mon Sep 17 00:00:00 2001 > > From: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> > > Date: Wed, 17 Mar 2010 19:49:22 +0300 > > Subject: [PATCH] vfs: reorganize inode chattr checks > > > > vmtrucate may fail due to IS_SWAPFILE flag or due to RLIMIT_FSIZE. > > In some situations it is not acceptable behaviour. Off course > > we can check IS_SWAPFILE before but what about RLIMIT_FSIZE? > > > > Let's divide newsize seting logic in two parts > > First which perform necessery size checks > > Second which does the work and can not fail. > > And perform size check before any inode modifications. > > > > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> > > --- > > fs/attr.c | 45 ++++++++++++++++++++++++++++++++++----------- > > fs/libfs.c | 8 ++++---- > > include/linux/fs.h | 5 +++-- > > 3 files changed, 41 insertions(+), 17 deletions(-) > > > > diff --git a/fs/attr.c b/fs/attr.c > > index 1bca479..1abaa05 100644 > > --- a/fs/attr.c > > +++ b/fs/attr.c > > @@ -18,7 +18,7 @@ > > /* Taken over from the old code... */ > > > > /* POSIX UID/GID verification for setting inode attributes. */ > > -int inode_change_ok(const struct inode *inode, struct iattr *attr) > > +int inode_change_posix_ok(const struct inode *inode, struct iattr *attr) > > { > > int retval = -EPERM; > > unsigned int ia_valid = attr->ia_valid; > > @@ -60,7 +60,7 @@ fine: > > error: > > return retval; > > } > > -EXPORT_SYMBOL(inode_change_ok); > > +EXPORT_SYMBOL(inode_change_posix_ok); > > > > /** > > * inode_newsize_ok - may this inode be truncated to a given size > > @@ -105,6 +105,19 @@ out_big: > > } > > EXPORT_SYMBOL(inode_newsize_ok); > > > > +/* Generic verification for setting inode attributes. */ > > +int inode_change_attr_ok(const struct inode *inode, struct iattr *attr) > > +{ > > + int ret; > > + ret = inode_change_posix_ok(inode, attr); > > + if (ret) > > + return ret; > > + if (attr->ia_valid & ATTR_SIZE) > > + ret = inode_newsize_ok(inode, attr->ia_size); > > + return ret; > > +} > > +EXPORT_SYMBOL(inode_change_attr_ok); > > + > > /** > > * generic_setattr - copy simple metadata updates into the generic inode > > * @inode: the inode to be updated > > @@ -241,17 +254,27 @@ int notify_change(struct dentry * dentry, struct iattr * attr) > > if (inode->i_op && inode->i_op->setattr) { > > error = inode->i_op->setattr(dentry, attr); > > } else { > > - error = inode_change_ok(inode, attr); > > - if (!error) { > > - if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) || > > - (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) > > - error = vfs_dq_transfer(inode, attr) ? > > - -EDQUOT : 0; > > - if (!error) > > - error = inode_setattr(inode, attr); > > + loff_t oldsize = inode->i_size; > > + error = inode_change_attr_ok(inode, attr); > > + if (error) > > + goto out; > > + if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) || > > + (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) { > > + error = vfs_dq_transfer(inode, attr) ? -EDQUOT : 0; > > + if (error) > > + goto out; > > + } > > + if (attr->ia_valid & ATTR_SIZE && > > + oldsize != attr->ia_size) { > > + i_size_write(inode, attr->ia_size); > > + truncate_pagecache(inode, oldsize, attr->ia_size); > > + if (inode->i_op->truncate) > > + inode->i_op->truncate(inode); > > } > > + generic_setattr(inode, attr); > > + mark_inode_dirty(inode); > > } > > - > > +out: > > if (ia_valid & ATTR_SIZE) > > up_write(&dentry->d_inode->i_alloc_sem); > > > > diff --git a/fs/libfs.c b/fs/libfs.c > > index a76934d..98f9a40 100644 > > --- a/fs/libfs.c > > +++ b/fs/libfs.c > > @@ -389,7 +389,7 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr) > > struct inode *inode = dentry->d_inode; > > int error; > > > > - error = inode_change_ok(inode, iattr); > > + error = inode_change_attr_ok(inode, iattr); > > if (error) > > return error; > > > > @@ -401,9 +401,9 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr) > > } > > > > if (iattr->ia_valid & ATTR_SIZE) { > > - error = simple_setsize(inode, iattr->ia_size); > > - if (error) > > - return error; > > + loff_t oldsize = inode->i_size; > > + i_size_write(inode, iattr->ia_size); > > + truncate_pagecache(inode, oldsize, iattr->ia_size); > > } > > > > generic_setattr(inode, iattr); > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 81c91d9..fb6809d 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -2388,9 +2388,10 @@ extern int buffer_migrate_page(struct address_space *, > > #else > > #define buffer_migrate_page NULL > > #endif > > - > > -extern int inode_change_ok(const struct inode *, struct iattr *); > > +#define inode_change_ok(inode, iattr) inode_change_posix_ok(inode, iattr) > > +extern int inode_change_posix_ok(const struct inode *, struct iattr *); > > extern int inode_newsize_ok(const struct inode *, loff_t offset); > > +extern int inode_change_attr_ok(const struct inode *, struct iattr *); > > extern int __must_check inode_setattr(struct inode *, const struct iattr *); > > extern void generic_setattr(struct inode *inode, const struct iattr *attr); > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html