Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1313641

Re: [RFC 02/15] vfs: Change all structures to support 64 bit time

From Dave Chinner <david@fromorbit.com>
Newsgroups linux.kernel
Subject Re: [RFC 02/15] vfs: Change all structures to support 64 bit time
Date 2016-01-21 00:10 +0100
Message-ID <qTa30-4VH-3@gated-at.bofh.it> (permalink)
References <qObsK-3Dn-7@gated-at.bofh.it> <qSx1D-38E-5@gated-at.bofh.it> <qSLxE-4AW-3@gated-at.bofh.it> <qSMWK-5DX-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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);
>  		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)) {
> -		inode->i_ctime = tv;
> +	    !timespec_equal(&ctime, &tv)) {
> +		inode->i_ctime = timespec_to_vfs_time(tv);
>  		ip->i_d.di_ctime.t_sec = tv.tv_sec;
>  		ip->i_d.di_ctime.t_nsec = tv.tv_nsec;
>  	}

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.
-- 
Dave Chinner
david@fromorbit.com

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-15 18:00 +0100
  Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Andreas Dilger <adilger@dilger.ca> - 2016-01-16 20:20 +0100
    Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-17 00:40 +0100
      Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Andreas Dilger <adilger@dilger.ca> - 2016-01-17 03:40 +0100
        Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit  time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-18 07:20 +0100
          Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-18 12:00 +0100
            [RFC 02/15] vfs: Change all structures to support 64 bit time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-18 18:50 +0100
              Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-18 21:00 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Dave Chinner <david@fromorbit.com> - 2016-01-18 22:20 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-18 22:50 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Dave Chinner <david@fromorbit.com> - 2016-01-19 02:50 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-19 06:30 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Dave Chinner <david@fromorbit.com> - 2016-01-19 22:00 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-19 23:30 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-20 06:20 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-20 16:10 +0100
                Re: [RFC 02/15] vfs: Change all structures to support 64 bit time Dave Chinner <david@fromorbit.com> - 2016-01-21 00:10 +0100
                Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time Arnd Bergmann <arnd@arndb.de> - 2016-01-21 00:20 +0100
                Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit  time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-01-27 07:30 +0100

csiph-web