| Message ID | 1408633667-24177-1-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Christoph, On Thu, Aug 21, 2014 at 11:07 AM, Christoph Hellwig <hch@lst.de> wrote: > The VFS handles attributes on truncate in a strange way, fix NFS to handle > it properly by copying a small code sniplet from XFS. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/nfs/inode.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 577a36f..5bbd991 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -505,8 +505,28 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr) > attr->ia_valid &= ~ATTR_MODE; > > if (attr->ia_valid & ATTR_SIZE) { > - if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode)) > + if (!S_ISREG(inode->i_mode) || > + attr->ia_size == i_size_read(inode)) > attr->ia_valid &= ~ATTR_SIZE; > + > + /* > + * Only change the c/mtime if we are changing the size or we > + * are explicitly asked to change it. This handles the > + * semantic difference between truncate() and ftruncate() as > + * implemented in the VFS. > + * > + * The regular truncate() case without ATTR_CTIME and > + * ATTR_MTIME is a special case where we need to update the > + * times despite not having these flags set. For all other > + * operations the VFS set these flags explicitly if it wants > + * a timestamp update. > + */ > + if (attr->ia_size != i_size_read(inode) && > + !(attr->ia_valid & (ATTR_CTIME | ATTR_MTIME))) { > + attr->ia_ctime = attr->ia_mtime = > + current_fs_time(inode->i_sb); > + attr->ia_valid |= ATTR_CTIME | ATTR_MTIME; > + } > } > We shouldn't need to set attr->ia_ctime/ia_mtime above. We should set the file time to the server clock time. Also note that instead of checking attr->ia_size != i_size_read(inode), the above should check attr->ia_valid & ATTR_SIZE due to the optimisation above.
On Thu, Aug 21, 2014 at 11:31:24AM -0400, Trond Myklebust wrote: > We shouldn't need to set attr->ia_ctime/ia_mtime above. We should set > the file time to the server clock time. True. > Also note that instead of checking attr->ia_size != > i_size_read(inode), the above should check attr->ia_valid & ATTR_SIZE > due to the optimisation above. Or we could just make it an if/else clause. I'll send out an updated patch ASAP. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 577a36f..5bbd991 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -505,8 +505,28 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr) attr->ia_valid &= ~ATTR_MODE; if (attr->ia_valid & ATTR_SIZE) { - if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode)) + if (!S_ISREG(inode->i_mode) || + attr->ia_size == i_size_read(inode)) attr->ia_valid &= ~ATTR_SIZE; + + /* + * Only change the c/mtime if we are changing the size or we + * are explicitly asked to change it. This handles the + * semantic difference between truncate() and ftruncate() as + * implemented in the VFS. + * + * The regular truncate() case without ATTR_CTIME and + * ATTR_MTIME is a special case where we need to update the + * times despite not having these flags set. For all other + * operations the VFS set these flags explicitly if it wants + * a timestamp update. + */ + if (attr->ia_size != i_size_read(inode) && + !(attr->ia_valid & (ATTR_CTIME | ATTR_MTIME))) { + attr->ia_ctime = attr->ia_mtime = + current_fs_time(inode->i_sb); + attr->ia_valid |= ATTR_CTIME | ATTR_MTIME; + } } /* Optimization: if the end result is no change, don't RPC */
The VFS handles attributes on truncate in a strange way, fix NFS to handle it properly by copying a small code sniplet from XFS. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/inode.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)