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

From: Dave Chinner
Date: Mon Jan 18 2016 - 20:41:09 EST


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

> > > 4. change the internal representation in one patch that changes those
> > > helpers along with the struct members.
> >
> > If you are talking about converting internal filesystem
> > representations to (e.g. CIFS fattr, NFS fattr, etc) then this is
> > wrong. Those filesystems are isolated and able to use timespecs
> > internally by step 3, and without protocol/format changes can't
> > support y2038k compliant dates. Hence fixing such problems is a
> > problem for the filesystem developers and is not an issue for the
> > VFS timestamp conversion.
>
> No, once we have the timespec_to_vfs_time helpers in all file
> systems, that change is just for VFS, and should not touch
> any file system specific code.

OK, just wanted to make clear, because to me "internal" tends to
mean "within a specific filesystem" whilst "generic" is used to
refer to things at the VFS layer...

Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx