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

Groups > linux.kernel > #1312507

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

Show all headers | View raw

On Mon, Jan 18, 2016 at 09:27:13PM -0800, Deepa Dinamani wrote:
> 
> 
> On Mon, Jan 18, 2016 at 5:38 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Jan 18, 2016 at 10:46:07PM +0100, Arnd Bergmann wrote:
> >> On Tuesday 19 January 2016 08:14:59 Dave Chinner wrote:
> >> > On Mon, Jan 18, 2016 at 08:53:22PM +0100, Arnd Bergmann wrote:
> >> > > 3. for each file system that uses struct timespec internally to pass
> >> > >    around inode timestamps, do one patch that adds a
> >> > >    timespec_to_inode_time() and vice versa, which gets defined like
> >> > >
> >> > > static inline struct timespec timespec_to_inode(struct timespec t)
> >> > > {
> >> > >   return t;
> >> > > }
> >> >
> >> > This works, and is much cleaner than propagating the macro nastiness
> >> > everywhere. IMO vfs_time_to_timespec()/timespec_to_vfs_time would be
> >> > better named as it describes the conversion exactly. I don't think
> >> > this is a huge patch, though - it's mainly the setattr/kstat
> >> > operations that need changing here.
> >>
> >> Good idea for the name.
> >>
> >> If you are ok with adding those helpers, then it can be done in small
> >> steps indeed. I was under the assumption that you didn't like any
> >> kind of abstraction of the type in struct inode at all.
> >
> > You're right, I don't like unnecessary abstractions.  I guess I've
> > not communicated the "convert timestamps at the edges, use native
> > timestamp types everywhere inside" structure very well, because type
> > conversion functions such as the above are an absolutely necessary
> > part of ensuring we don't need abstractions in the core code... :P
> 
> 
> Let's back out a bit and consider a few changes with the suggested "abstraction":
> 
> original code:
> 
> extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
> __le16 __time, __le16 __date, u8 time_cs);
> 
> fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
> 
> becomes ugly
> 
> extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec64 *ts,
> __le16 __time, __le16 __date, u8 time_cs);
> 
> struct timespec64 mtime = vfs_time_to_timespec64(i_mtime, inode);
> fat_time_fat2unix(sbi, &mtime, de->time, de->date, 0);

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....

I think you're making a mountain out of a molehill. Most filesystems
will be unchanged except for s/timespec/timespec64/ as they store
values directly into timespec members when encoding/decoding. There
is no need for timestamp conversion in places like this - you're
simply not looking deep enough and applying the conversion at the
wrong layer.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Back to linux.kernel | Previous | Next — Previous 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