On Tue, Jan 19, 2016 at 11:25:02PM +0100, Arnd Bergmann wrote:
On Wednesday 20 January 2016 07:49:46 Dave Chinner wrote:
You're doing it wrong. fat_time_fat2unix() still gets passed &inode->i_mtime, and the function prototype is changed to a timespec64. *Nothing else needs to change*, because fat_time_fat2unix() does it own calculations and then stores the time directly into the timespec structure members....
Any idea how to improve this somewhat lacking patch?
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index b97f1df910ab..7fbb07dcad36 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -68,22 +68,24 @@ xfs_trans_ichgtime( int flags) { struct inode *inode = VFS_I(ip);
- struct timespec tv;
- struct timespec tv, mtime, ctime;
ASSERT(tp); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- tv = current_fs_time(inode->i_sb);
- tv = vfs_time_to_timespec(current_fs_time(inode->i_sb));
- mtime = vfs_time_to_timespec(inode->i_mtime);
- ctime = vfs_time_to_timespec(inode->i_ctime);
if ((flags & XFS_ICHGTIME_MOD) &&
!timespec_equal(&inode->i_mtime, &tv)) {inode->i_mtime = tv;
!timespec_equal(&mtime, &tv)) {inode->i_mtime = timespec_to_vfs_time(tv);
!timespec_equal(&inode->i_ctime, &tv)) {inode->i_ctime = tv;
!timespec_equal(&ctime, &tv)) {inode->i_ctime = timespec_to_vfs_time(tv);
WTF? That's insane and completely unnecessary. It's even worse than the FAT example I've already pointed out was just fucking wrong.
The only change this function requires is:
The way that Deepa suggests I think would turn out as:
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index b97f1df910ab..54fc3c41047a 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -68,7 +68,7 @@ xfs_trans_ichgtime( int flags) { struct inode *inode = VFS_I(ip);
- struct timespec tv;
- struct vfs_time tv;
+ struct timespec64 tv;
ASSERT(tp); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); @@ -76,13 +76,13 @@ xfs_trans_ichgtime( tv = current_fs_time(inode->i_sb); if ((flags & XFS_ICHGTIME_MOD) &&
!timespec_equal(&inode->i_mtime, &tv)) {
!vfs_time_equal(&inode->i_mtime, &tv)) {
+ !timespec64_equal(&inode->i_mtime, &tv)) {
inode->i_mtime = tv; ip->i_d.di_mtime.t_sec = tv.tv_sec; ip->i_d.di_mtime.t_nsec = tv.tv_nsec;} if ((flags & XFS_ICHGTIME_CHG) &&
!timespec_equal(&inode->i_ctime, &tv)) {
!vfs_time_equal(&inode->i_ctime, &tv)) {
+ !timespec64_equal(&inode->i_ctime, &tv)) {
inode->i_ctime = tv; ip->i_d.di_ctime.t_sec = tv.tv_sec; ip->i_d.di_ctime.t_nsec = tv.tv_nsec;
which I would much prefer here.
IOWs, you're now finally suggesting doing the *simple conversion* I've been suggesting needs to be made *since the start* of this long, frustrating thread, except you *still want to abstract the timestamp unnecessarily*.
For the last time: use timespec64 directly, do not abstract it in any way.
-Dave.