Hi,
On Sun, 2005-10-30 at 23:48 -0800, Andrew Morton wrote:
> NeilBrown <[email protected]> wrote:
> > According to Posix and SUS, truncate(2) and ftruncate(2) only update
> > ctime and mtime if the size actually changes. Linux doesn't currently
> > obey this.
> >
> > There is no need to test the size under i_sem, as loosing any race
> > will not make a noticable different the mtime or ctime.
>
> Well if there's a race then the file may end up not being truncated after
> this patch is applied. But that could have happened anwyay, so I don't see
> a need for i_sem synchronisation either.
>
> > (According to SUS, truncate and ftruncate 'may' clear setuid/setgid
> > as well, currently we don't. Should we?
> > )
> >
> >
> > Signed-off-by: Neil Brown <[email protected]>
> >
> > ### Diffstat output
> > ./fs/open.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff ./fs/open.c~current~ ./fs/open.c
> > --- ./fs/open.c~current~ 2005-10-31 16:22:44.000000000 +1100
> > +++ ./fs/open.c 2005-10-31 16:22:44.000000000 +1100
> > @@ -260,7 +260,8 @@ static inline long do_sys_truncate(const
> > goto dput_and_out;
> >
> > error = locks_verify_truncate(inode, NULL, length);
> > - if (!error) {
> > + if (!error &&
> > + length != i_size_read(dentry->d_inode)) {
>
> Odd code layout?
>
> > DQUOT_INIT(inode);
> > error = do_truncate(nd.dentry, length);
> > }
> > @@ -313,7 +314,8 @@ static inline long do_sys_ftruncate(unsi
> > goto out_putf;
> >
> > error = locks_verify_truncate(inode, file, length);
> > - if (!error)
> > + if (!error &&
> > + length != i_size_read(dentry->d_inode))
> > error = do_truncate(dentry, length);
> > out_putf:
> > fput(file);
>
> This partially obsoletes the similar optimisation in inode_setattr(). I
> guess the optimisation there retains some usefulness for O_TRUNC opens of
> zero-length files, but for symettry and micro-efficiency, perhaps we should
> remvoe the inode_setattr() test and check for i_size==0 in may_open()?
Sounds like a good idea. That does simplify inode_setattr() nicely...
Signed-off-by: Anton Altaparmakov <[email protected]>
--- linux-2.6/fs/attr.c.old 2005-10-31 09:29:38.000000000 +0000
+++ linux-2.6/fs/attr.c 2005-10-31 09:30:39.000000000 +0000
@@ -70,19 +70,10 @@ int inode_setattr(struct inode * inode,
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;
- }
+ error = vmtruncate(inode, attr->ia_size);
+ if (error || (ia_valid == ATTR_SIZE))
+ goto out;
}
-
if (ia_valid & ATTR_UID)
inode->i_uid = attr->ia_uid;
if (ia_valid & ATTR_GID)
btw. Is it actually correct that we "goto out;" when "ia_valid ==
ATTR_SIZE"? That way we skip the mark_inode_dirty() call just before
the "out" label...
For ntfs at least that is fine because ntfs does an
"inode_update_time(inode, 1)" unconditionally in ntfs_truncate() even
when the size has not changed which calls mark_inode_dirty_sync() and
when the size changes it also does a "__mark_inode_dirty(inode,
I_DIRTY_SYNC | I_DIRTY_DATASYNC);" but I am not sure all filesystems are
fine in that respect?
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
-
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]
