On Monday October 31, [email protected] wrote:
>
> What we can, however, do is to ensure that truncate() and ftruncate()
> only set ATTR_SIZE, but ensure that may_open() sets ATTR_MTIME|
> ATTR_CTIME as well.
Thanks. This makes lots of sense, in that it gives power to the
filesystem. We should keep the optimisations in placed where the
filesystem can over-ride them, just as inode_setattr.
So, here is a revised patch.
Comment?
Thanks,
NeilBrown
------------------
Fix some problems with truncate and mtime semantics.
SUS requires that when truncating a file to the size that it currently
is:
truncate and ftruncate should NOT modify ctime or mtime
O_TRUNC SHOULD modify ctime and mtime.
Currently mtime and ctime are always modified on most local
filesystems (side effect of ->truncate) or never modified (on NFS).
With this patch:
ATTR_CTIME|ATTR_MTIME are sent with ATTR_SIZE precisely when
an update of these times is required whether size changes or not
(via a new argument to do_truncate). This allows NFS to do
the right thing for O_TRUNC.
inode_setattr no longer forces ATTR_MTIME|ATTR_CTIME when the ATTR_SIZE
sets the size to it's current value. This allows local filesystems
to do the right thing for f?truncate.
Also, the logic in inode_setattr is changed a bit so there are two
return points. One returns the error from vmtruncate if it failed,
the other returns 0 (there can be no other failure).
Finally, if vmtruncate succeeds, and ATTR_SIZE is the only change
requested, we now fall-through and mark_inode_dirty. If a filesystem
did not have a ->truncate function, then vmtruncate will have changed
i_size, without marking the inode as 'dirty', and I think this is wrong.
Signed-off-by: Neil Brown <[email protected]>
### Diffstat output
./fs/attr.c | 22 +++++++---------------
./fs/exec.c | 2 +-
./fs/namei.c | 2 +-
./fs/open.c | 8 ++++----
./include/linux/fs.h | 2 +-
5 files changed, 14 insertions(+), 22 deletions(-)
diff ./fs/attr.c~current~ ./fs/attr.c
--- ./fs/attr.c~current~ 2005-11-01 09:42:22.000000000 +1100
+++ ./fs/attr.c 2005-11-01 09:22:40.000000000 +1100
@@ -67,20 +67,12 @@ EXPORT_SYMBOL(inode_change_ok);
int inode_setattr(struct inode * inode, struct iattr * attr)
{
unsigned int ia_valid = attr->ia_valid;
- int error = 0;
- if (ia_valid & ATTR_SIZE) {
- if (attr->ia_size != i_size_read(inode)) {
- error = vmtruncate(inode, attr->ia_size);
- if (error || (ia_valid == ATTR_SIZE))
- goto out;
- } else {
- /*
- * We skipped the truncate but must still update
- * timestamps
- */
- ia_valid |= ATTR_MTIME|ATTR_CTIME;
- }
+ if (ia_valid & ATTR_SIZE &&
+ attr->ia_size != i_size_read(inode)) {
+ int error = vmtruncate(inode, attr->ia_size);
+ if (error)
+ return error;
}
if (ia_valid & ATTR_UID)
@@ -104,8 +96,8 @@ int inode_setattr(struct inode * inode,
inode->i_mode = mode;
}
mark_inode_dirty(inode);
-out:
- return error;
+
+ return 0;
}
EXPORT_SYMBOL(inode_setattr);
diff ./fs/exec.c~current~ ./fs/exec.c
--- ./fs/exec.c~current~ 2005-11-01 09:42:22.000000000 +1100
+++ ./fs/exec.c 2005-11-01 09:19:35.000000000 +1100
@@ -1510,7 +1510,7 @@ int do_coredump(long signr, int exit_cod
goto close_fail;
if (!file->f_op->write)
goto close_fail;
- if (do_truncate(file->f_dentry, 0) != 0)
+ if (do_truncate(file->f_dentry, 0, 0) != 0)
goto close_fail;
retval = binfmt->core_dump(signr, regs, file);
diff ./fs/namei.c~current~ ./fs/namei.c
--- ./fs/namei.c~current~ 2005-11-01 09:42:22.000000000 +1100
+++ ./fs/namei.c 2005-11-01 09:24:34.000000000 +1100
@@ -1459,7 +1459,7 @@ int may_open(struct nameidata *nd, int a
if (!error) {
DQUOT_INIT(inode);
- error = do_truncate(dentry, 0);
+ error = do_truncate(dentry, 0, ATTR_MTIME|ATTR_CTIME);
}
put_write_access(inode);
if (error)
diff ./fs/open.c~current~ ./fs/open.c
--- ./fs/open.c~current~ 2005-11-01 09:42:22.000000000 +1100
+++ ./fs/open.c 2005-11-01 09:42:26.000000000 +1100
@@ -194,7 +194,7 @@ out:
return error;
}
-int do_truncate(struct dentry *dentry, loff_t length)
+int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs)
{
int err;
struct iattr newattrs;
@@ -204,7 +204,7 @@ int do_truncate(struct dentry *dentry, l
return -EINVAL;
newattrs.ia_size = length;
- newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
+ newattrs.ia_valid = ATTR_SIZE | time_attrs;
down(&dentry->d_inode->i_sem);
err = notify_change(dentry, &newattrs);
@@ -262,7 +262,7 @@ static inline long do_sys_truncate(const
error = locks_verify_truncate(inode, NULL, length);
if (!error) {
DQUOT_INIT(inode);
- error = do_truncate(nd.dentry, length);
+ error = do_truncate(nd.dentry, length, 0);
}
put_write_access(inode);
@@ -314,7 +314,7 @@ static inline long do_sys_ftruncate(unsi
error = locks_verify_truncate(inode, file, length);
if (!error)
- error = do_truncate(dentry, length);
+ error = do_truncate(dentry, length, 0);
out_putf:
fput(file);
out:
diff ./include/linux/fs.h~current~ ./include/linux/fs.h
--- ./include/linux/fs.h~current~ 2005-11-01 09:42:22.000000000 +1100
+++ ./include/linux/fs.h 2005-11-01 09:18:35.000000000 +1100
@@ -1295,7 +1295,7 @@ static inline int break_lease(struct ino
/* fs/open.c */
-extern int do_truncate(struct dentry *, loff_t start);
+extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs);
extern long do_sys_open(const char __user *filename, int flags, int mode);
extern struct file *filp_open(const char *, int, int);
extern struct file * dentry_open(struct dentry *, struct vfsmount *, int);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
