Timothy Shimmin wrote:
Hi Roger,
...
I don't like all these inconsistencies.
Take a look at the attached patch relative to the current cvs (it's a bit
big to put
inline). The basic problem is it's currently unclear when to set the times
from
va_atime etc. and when to set them to the current time. So I've used the
already
defined XFS_AT_UPDxTIME flags to indicate that a time should be set to 'now'
and XFS_AT_xTIME to mean set it using va_xtime. This seems to fit well with
the current code and I wonder if that's how it was meant to work in the
first
place. I've also removed the now redundant ATTR_UTIME flag and pulled
the null truncate to the top, which simplifies things.
One query: in both xfs_iops.c/xfs_vn_setattr and
xfs_dm.c/xfs_dm_set_fileattr the
ATIME branch sets the inode's atime directly. This is probably something to
do with
the comment above xfs_iops.c/xfs_ichgtime ('to make sure the access time
update
will take') but it could probably be handled better.
BTW, your locking looks wrong - it appears you don't unlock when the
file is non-zero size.
Oops...
--
Roger
xfs_setattr.patch
Description: Binary data
|