On Monday October 31, [email protected] wrote:
> >
> > 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?
>
I think the 'goto' is fine as presumably an error from vmtruncate
means that nothing was changed, and as there are no other attr bits,
there is no need to mark_inode_dirty.
However, there always will be other ATTR bits as whenever we set
ATTR_SIZE (do_truncate and nfsd_setattr) we also set ATTR_CTIME,
so this "optimisation" is just a waste of space.
Best make it:
> if (ia_valid & ATTR_SIZE)
> error = vmtruncate(inode, attr->ia_size);
and assume that the filesystem's ->truncate or ->setattr will still
set mtime if the size doesn't change (->truncate would have a hard
time knowing if it has changed or not, so it has to set mtime
unconditionally if it exists .. ->setattr... probably does the right
thing)
NeilBrown
-
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]
