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

Groups > linux.kernel > #1303308 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

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

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

The current representation of inode times in struct inode, struct iattr,
and struct kstat use struct timespec. timespec is not y2038 safe.

Use scalar data types (seconds and nanoseconds stored separately) to
represent timestamps in struct inode in order to maintain same size for
times across 32 bit and 64 bit architectures.
In addition, lay them out such that they are packed on a naturally
aligned boundary on 64 bit arch as 4 bytes are used to store nsec.
This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes.
This will help save RAM space as inode structure is cached in memory.
The other structures are transient and do not benefit from these
changes.

Add accessors for inode timestamps.
These provide a way to access the time field members.
Accessors abstract the timestamp representation so that any logic
to convert between the struct inode timestamps and other interfaces
can be placed here.
The plan is to globally change all references to these types through
these accessors only. So when the actual internal representation
changes, it will be transparent to the outside world.
This can be extended to add code to validate the inode times that
are being set.
Macros are chosen as accessors rather than functions because we can
condense 3 {a,c,m} time functions into a single macro. After we agree
on an approach, the implementation could be changed to use static
inline functions if it suits more.

Add inode_timespec aliases to help convert kstat and iattr times to use
64 bit times. These hide the internal data type.
Use uapi exposed data types here to keep minimal timstamp data type
conversions in API's interfacing with vfs.

After the CONFIG_FS_USES_64BIT_TIME is enabled, all inode_timespec
aliases will be removed and timespec64 data types and API's will
be used directly.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 include/linux/fs.h     | 55 +++++++++++++++++++++++++++++++++++++++++++-------
 include/linux/stat.h   |  6 +++---
 include/linux/time64.h | 21 +++++++++++++++++++
 3 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 12ba937..b9f3cee 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -245,13 +245,13 @@ typedef void (dax_iodone_t)(struct buffer_head *bh_map, int uptodate);
  */
 struct iattr {
 	unsigned int	ia_valid;
-	umode_t		ia_mode;
-	kuid_t		ia_uid;
-	kgid_t		ia_gid;
-	loff_t		ia_size;
-	struct timespec	ia_atime;
-	struct timespec	ia_mtime;
-	struct timespec	ia_ctime;
+	umode_t			ia_mode;
+	kuid_t			ia_uid;
+	kgid_t			ia_gid;
+	loff_t			ia_size;
+	struct inode_timespec	ia_atime;
+	struct inode_timespec	ia_mtime;
+	struct inode_timespec	ia_ctime;
 
 	/*
 	 * Not an attribute, but an auxiliary info for filesystems wanting to
@@ -616,9 +616,18 @@ struct inode {
 	};
 	dev_t			i_rdev;
 	loff_t			i_size;
+#ifdef CONFIG_FS_USES_64BIT_TIME
+	time64_t		i_atime_sec;
+	time64_t		i_mtime_sec;
+	time64_t		i_ctime_sec;
+	s32			i_atime_nsec;
+	s32			i_mtime_nsec;
+	s32			i_ctime_nsec;
+#else
 	struct timespec		i_atime;
 	struct timespec		i_mtime;
 	struct timespec		i_ctime;
+#endif
 	spinlock_t		i_lock;	/* i_blocks, i_bytes, maybe i_size */
 	unsigned short          i_bytes;
 	unsigned int		i_blkbits;
@@ -679,6 +688,38 @@ struct inode {
 	void			*i_private; /* fs or device private pointer */
 };
 
+#ifdef CONFIG_FS_USES_64BIT_TIME
+
+#define VFS_INODE_SET_XTIME(xtime, inode, ts64)			\
+	do {								\
+		struct inode_timespec __ts = (ts64);			\
+		(inode)->xtime##_sec = __ts.tv_sec;			\
+		(inode)->xtime##_nsec = __ts.tv_nsec;			\
+	} while (0)
+
+#define VFS_INODE_GET_XTIME(xtime, inode)			\
+	(struct timespec64){.tv_sec = (inode)->xtime##_sec,	\
+			    .tv_nsec = (inode)->xtime##_nsec}
+
+#else
+
+#define VFS_INODE_SET_XTIME(xtime, inode, ts)			\
+	((inode)->xtime = (ts))
+
+#define VFS_INODE_GET_XTIME(xtime, inode)       \
+	((inode)->xtime)
+
+#endif
+
+#define VFS_INODE_SWAP_XTIME(xtime, inode1, inode2)		\
+	do {							\
+		struct inode_timespec __ts =			\
+			VFS_INODE_GET_XTIME(xtime, inode1);	\
+		VFS_INODE_SET_XTIME(xtime, inode1,		\
+			VFS_INODE_GET_XTIME(xtime, inode2));	\
+		VFS_INODE_SET_XTIME(xtime, inode2, __ts);	\
+	} while (0)
+
 static inline int inode_unhashed(struct inode *inode)
 {
 	return hlist_unhashed(&inode->i_hash);
diff --git a/include/linux/stat.h b/include/linux/stat.h
index 075cb0c..559983f 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -27,9 +27,9 @@ struct kstat {
 	kgid_t		gid;
 	dev_t		rdev;
 	loff_t		size;
-	struct timespec  atime;
-	struct timespec	mtime;
-	struct timespec	ctime;
+	struct inode_timespec	atime;
+	struct inode_timespec	mtime;
+	struct inode_timespec	ctime;
 	unsigned long	blksize;
 	unsigned long long	blocks;
 };
diff --git a/include/linux/time64.h b/include/linux/time64.h
index 367d5af..be98201 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -26,6 +26,27 @@ struct itimerspec64 {
 
 #endif
 
+#ifdef CONFIG_FS_USES_64BIT_TIME
+
+/* Place holder defines until CONFIG_FS_USES_64BIT_TIME
+ * is enabled.
+ * timespec64 data type and functions will be used at that
+ * time directly and these defines will be deleted.
+ */
+#define inode_timespec timespec64
+
+#define inode_timespec_compare	timespec64_compare
+#define inode_timespec_equal	timespec64_equal
+
+#else
+
+#define inode_timespec timespec
+
+#define inode_timespec_compare	timespec_compare
+#define inode_timespec_equal	timespec_equal
+
+#endif
+
 /* Parameters used to convert the timespec values: */
 #define MSEC_PER_SEC	1000L
 #define USEC_PER_MSEC	1000L
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1305734

On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> The current representation of inode times in struct inode, struct iattr,
> and struct kstat use struct timespec. timespec is not y2038 safe.
> 
> Use scalar data types (seconds and nanoseconds stored separately) to
> represent timestamps in struct inode in order to maintain same size for
> times across 32 bit and 64 bit architectures.
> In addition, lay them out such that they are packed on a naturally
> aligned boundary on 64 bit arch as 4 bytes are used to store nsec.
> This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes.
> This will help save RAM space as inode structure is cached in memory.
> The other structures are transient and do not benefit from these
> changes.

IMO, this decisions sends the patch series immediately down the
wrong path. TO me, this is a severe case of premature optimisation
because everything gets way more complex just to save those 8 bytes,
especially as those holes can be filled simply by changing the
variable declaration order in the structure and adding a simple
comment.

And, really, I don't like those VFS_INODE_[GS]ET_XTIME macros at
all; you've got to touch lots of code(*), making it all shouty and
harder to read.  They seem only to exist because of the above
structural change requires an abstract timestamp accessor while
CONFIG_FS_USES_64BIT_TIME exists. Given that goes away at the end o
the series, so should the macro - if we use a struct timespec64 in
the first place, it isn't even necessary as a temporary construct.

(*) I note you haven't touched XFS, which means you've probably
broken lots of other filesystem code. e.g. in XFS, functions like
xfs_vn_getattr() and xfs_vn_update_time() access inode->i_[acm]time
directly and hence are not going to compile after this patch series.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1306998

> On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
>
>> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
>> The current representation of inode times in struct inode, struct iattr,
>> and struct kstat use struct timespec. timespec is not y2038 safe.
>>
>> Use scalar data types (seconds and nanoseconds stored separately) to
>> represent timestamps in struct inode in order to maintain same size for
>> times across 32 bit and 64 bit architectures.
>> In addition, lay them out such that they are packed on a naturally
>> aligned boundary on 64 bit arch as 4 bytes are used to store nsec.
>> This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes.
>> This will help save RAM space as inode structure is cached in memory.
>> The other structures are transient and do not benefit from these
>> changes.
>
> IMO, this decisions sends the patch series immediately down the
> wrong path.

There are other things the patch does that I would like to get comments
on: inode_timespec aliases, range check, individual fs changes etc.
These are independent of the inode timestamp representation changes.

>TO me, this is a severe case of premature optimisation
> because everything gets way more complex just to save those 8 bytes,
> especially as those holes can be filled simply by changing the
> variable declaration order in the structure and adding a simple
> comment.

I had tried rearranging the structure and the pahole tool does not show
any difference unless you pack and align the struct to 4 bytes on 64
bit arch.  The change actually saves 16 bytes on x86_64 and adds 12
bytes on i386.

Here is the breakdown for struct inode before and after the patch:

x86_64:
/* size: 544, cachelines: 9, members: 44 */           |
/* size: 528, cachelines: 9, members: 47 */
/* sum members: 534, holes: 3, sum holes: 10 */       |
/* sum members: 522, holes: 2, sum holes: 6 */

i386:
/* size: 328, cachelines: 6, members: 45 */           |
/* size: 340, cachelines: 6, members: 48 */
/* sum members: 326, holes: 1, sum holes: 2 */        |
/* sum members: 338, holes: 1, sum holes: 2 */

According to /proc/slabinfo I estimated savings of 4MB on a lightly
loaded system.

> And, really, I don't like those VFS_INODE_[GS]ET_XTIME macros at
> all; you've got to touch lots of code(*), making it all shouty and
> harder to read.  They seem only to exist because of the above
> structural change requires an abstract timestamp accessor while
> CONFIG_FS_USES_64BIT_TIME exists.
> Given that goes away at the end o
> the series, so should the macro - if we use a struct timespec64 in
> the first place, it isn't even necessary as a temporary construct

timespec64 was the first option considered here.  The problem with using
timespec64 is the long data type to represent nsec.  If it were possible
to change timespec64 nsec to int data type then it might be okay  to use
that if we are not worried about holes.  I do not see why time stamps
should have different representations on a 32 bit vs a 64 bit arch.
This left us with the option define a new data type to represent
timestamps.  I agreed with the concerns on the earlier RFC series that
there are already very many data types to represent time in the kernel.
So this left me with the option of using scalar types to represent these.
The scalar types were not used for optimization. They just happened to
serve that purpose as well.  This could be in a follow on patch, but as
long as we are changing the representation everywhere, I don't see why
there should be an intermediate step to change it to timespec64 only to
change it to this representation later.

As far as accessors are concerned, there already are accessors in the
VFS: generic_fillattr() and setattr_copy(). The problem really is that
there is more than one way of updating these attributes(timestamps in
this particular case). The side effect of this is that we don't always
call timespec_trunc() before assigning timestamps which can lead to
inconsistencies between on disk and in memory inode timestamps. Also,
since these also touch other attributes, these become more restrictive.
The accessors were an idea to streamline all accesses to timestamps in
inode.  Right now the accessor macros also figure out if the timestamps
were clamped and then call the registered callback.  But, this can be
extended to include fs_time_trunc() and then all the end users can
just use these and not worry about the right granularity or range.
As the commit text says, these can be changed to inline functions to
avoid shouty case.

> (*) I note you haven't touched XFS, which means you've probably
> broken lots of other filesystem code. e.g. in XFS, functions like
> xfs_vn_getattr() and xfs_vn_update_time() access inode->i_[acm]time
> directly and hence are not going to compile after this patch series.

I think I should have explained this more in my cover letter, as this
has come up twice now.  Patches 1-7 are the only ones that are relevant
and compiled and tested.  These change three example filesystems as an
illustration of the proposed solution.  Of course, every filesystem will
have to be changed similarly before patches 8-15 and a few more to change
additional filesystems to use timespec64 can be merged.  Patches 8-15 were
included merely to provide a complete picture, as I thought patches
explained the concept better than words only. These have not even been
compiled, as these are for illustration purposes as noted in the cover
letter.

-Deepa

[toc] | [prev] | [next] | [standalone]

#1307098

On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> >
> >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> >> The current representation of inode times in struct inode, struct iattr,
> >> and struct kstat use struct timespec. timespec is not y2038 safe.
> >>
> >> Use scalar data types (seconds and nanoseconds stored separately) to
> >> represent timestamps in struct inode in order to maintain same size for
> >> times across 32 bit and 64 bit architectures.
> >> In addition, lay them out such that they are packed on a naturally
> >> aligned boundary on 64 bit arch as 4 bytes are used to store nsec.
> >> This makes each tuple(sec, nscec) use 12 bytes instead of 16 bytes.
> >> This will help save RAM space as inode structure is cached in memory.
> >> The other structures are transient and do not benefit from these
> >> changes.
> >
> > IMO, this decisions sends the patch series immediately down the
> > wrong path.
> 
> There are other things the patch does that I would like to get comments
> on: inode_timespec aliases, range check, individual fs changes etc.
> These are independent of the inode timestamp representation changes.

The inode_timespec alias is part of the problem - AFAICT it exists
because you need a representation that is independent of both the
old and new in-memory structures.

The valid timestamp range stuff in the superblock is absolutely
necessary, but that's something that can be done completely
independently to the changes for supporting a differnet time storage
format.

And the fs changes cannot really be commented on until the VFs time
representation is sorted out properly...

> >TO me, this is a severe case of premature optimisation
> > because everything gets way more complex just to save those 8 bytes,
> > especially as those holes can be filled simply by changing the
> > variable declaration order in the structure and adding a simple
> > comment.
> 
> I had tried rearranging the structure and the pahole tool does not show
> any difference unless you pack and align the struct to 4 bytes on 64
> bit arch.  The change actually saves 16 bytes on x86_64 and adds 12
> bytes on i386.
> 
> Here is the breakdown for struct inode before and after the patch:
> 
> x86_64:
> /* size: 544, cachelines: 9, members: 44 */           |
> /* size: 528, cachelines: 9, members: 47 */
> /* sum members: 534, holes: 3, sum holes: 10 */       |
> /* sum members: 522, holes: 2, sum holes: 6 */
> 
> i386:
> /* size: 328, cachelines: 6, members: 45 */           |
> /* size: 340, cachelines: 6, members: 48 */
> /* sum members: 326, holes: 1, sum holes: 2 */        |
> /* sum members: 338, holes: 1, sum holes: 2 */
> 
> According to /proc/slabinfo I estimated savings of 4MB on a lightly
> loaded system.
> 
> > And, really, I don't like those VFS_INODE_[GS]ET_XTIME macros at
> > all; you've got to touch lots of code(*), making it all shouty and
> > harder to read.  They seem only to exist because of the above
> > structural change requires an abstract timestamp accessor while
> > CONFIG_FS_USES_64BIT_TIME exists.
> > Given that goes away at the end o
> > the series, so should the macro - if we use a struct timespec64 in
> > the first place, it isn't even necessary as a temporary construct
> 
> timespec64 was the first option considered here.  The problem with using
> timespec64 is the long data type to represent nsec.  If it were possible
> to change timespec64 nsec to int data type then it might be okay  to use
> that if we are not worried about holes.  I do not see why time stamps
> should have different representations on a 32 bit vs a 64 bit arch.

What's it matter? iot's irrelevant to the problem at hand.

Besides, have you looked at the existing timestamp definitions? they
use a struct timespec, which on a 64 bit system:

        struct timespec            i_atime;              /*    88    16 */
        struct timespec            i_mtime;              /*   104    16 */
        struct timespec            i_ctime;              /*   120    16 */

use 2 longs and are identical to a timespec64 in everything but
name. These should just be changed to a timespec64, and we suck up
the fact it increases the size of the 32 bit inode as we have to
increase it's size to support time > y2038 anyway.

This is what I meant about premature optimisation - you've got a
wonderfully complex solution to a problem that we don't need to
solve to support timestamps >y2038. It's also why it goes down the
wrong path at this point - most of the changes are not necessary if
all we need to do is a simple timespec -> timespec64 type change and
the addition timestamp range limiting in the existing truncation
function...

> The problem really is that
> there is more than one way of updating these attributes(timestamps in
> this particular case). The side effect of this is that we don't always
> call timespec_trunc() before assigning timestamps which can lead to
> inconsistencies between on disk and in memory inode timestamps.

That's a problem that can be fixed independently of y2038 support.
Indeed, we can be quite lazy about updating timestamps - by intent
and design we usually have different timestamps in memory compared
to on disk, which is one of the reasons why there are so many
different ways to change and update timestamps....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Tuesday 12 January 2016 19:29:57 Dave Chinner wrote:
> 
> This is what I meant about premature optimisation - you've got a
> wonderfully complex solution to a problem that we don't need to
> solve to support timestamps >y2038. It's also why it goes down the
> wrong path at this point - most of the changes are not necessary if
> all we need to do is a simple timespec -> timespec64 type change and
> the addition timestamp range limiting in the existing truncation
> function...

I originally suggested doing the split representation because I
was worried about the downsides of using timespec64 on 32-bit systems
after looking at actual memory consumption on my test box.

At this moment, I have a total of 145712700 inodes in memory on a machine
with 64GB ram, saving 12 bytes on each amounts to a total of 145MB. I think
it was more than that when I first looked, so it's between 0.2% and 0.3%
of savings in total memory, which is certainly worth discussing about,
given the renewed interest in conserving RAM in general. If we want to
save this memory, then doing it at the same time as the timespec64 conversion
is the right time so we don't need to touch every file twice.

One point that I had not considered though is on the 32-bit systems we
are talking about, not only is RAM much smaller, but also there would be
a smaller fraction of RAM available to store inodes, so there is not as
much to gain.

	Arnd

[toc] | [prev] | [next] | [standalone]

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

On Tue, Jan 12, 2016 at 10:27:07AM +0100, Arnd Bergmann wrote:
> On Tuesday 12 January 2016 19:29:57 Dave Chinner wrote:
> > 
> > This is what I meant about premature optimisation - you've got a
> > wonderfully complex solution to a problem that we don't need to
> > solve to support timestamps >y2038. It's also why it goes down the
> > wrong path at this point - most of the changes are not necessary if
> > all we need to do is a simple timespec -> timespec64 type change and
> > the addition timestamp range limiting in the existing truncation
> > function...
> 
> I originally suggested doing the split representation because I
> was worried about the downsides of using timespec64 on 32-bit systems
> after looking at actual memory consumption on my test box.
> 
> At this moment, I have a total of 145712700 inodes in memory on a machine

Is that all? :P

> with 64GB ram, saving 12 bytes on each amounts to a total of 145MB.

I just posted a patchset that knocks 104 bytes off the XFS inode
(~12% reduction in size).  We need the changes in that patchset to
sanely support >y2038k support in XFS, and it means we now won't
need to grow the XFS inode to add that support, either.

> I think
> it was more than that when I first looked, so it's between 0.2% and 0.3%
> of savings in total memory, which is certainly worth discussing about,
> given the renewed interest in conserving RAM in general.  If we want to
> save this memory, then doing it at the same time as the timespec64 conversion
> is the right time so we don't need to touch every file twice.

You just uttered the key words: "If we want to save this memory"

So let's stop conflating two different lines of development because
we only actually *need* y2038k support.

The fact we haven't made timestamp space optimisations means
that nobody has thought it necessary or worthwhile. y2038k support
doesn't change the landscape under which we might consider the
optimisation, so we need to determine if the benefit outweighs the
cost in terms of code complexity and maintainability.

So separate the two changes - make the y2038k change simple and
obviously correct first by changing everything to timespec64. Then it
won't get delayed by bikeshedding about an optimisation of that is
of questionable benefit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Wednesday 13 January 2016 17:27:16 Dave Chinner wrote:
> > I think
> > it was more than that when I first looked, so it's between 0.2% and 0.3%
> > of savings in total memory, which is certainly worth discussing about,
> > given the renewed interest in conserving RAM in general.  If we want to
> > save this memory, then doing it at the same time as the timespec64 conversion
> > is the right time so we don't need to touch every file twice.
> 
> You just uttered the key words: "If we want to save this memory"
> 
> So let's stop conflating two different lines of development because
> we only actually *need* y2038k support.
> 
> The fact we haven't made timestamp space optimisations means
> that nobody has thought it necessary or worthwhile. y2038k support
> doesn't change the landscape under which we might consider the
> optimisation, so we need to determine if the benefit outweighs the
> cost in terms of code complexity and maintainability.
> 
> So separate the two changes - make the y2038k change simple and
> obviously correct first by changing everything to timespec64. Then it
> won't get delayed by bikeshedding about an optimisation of that is
> of questionable benefit.

Fine with me. I think Deepa already started simplifying the series
already. I agree that for 64-bit machines, there is no need to optimize
that code now, since we are not regressing in terms of memory size.

For 32-bit machines, we are regressing anyway, the question is whether
it's by 12 or 24 bytes per inode. Let me try to estimate the worse-case
scenario here: let's assume that we have 1GB of RAM (anything more
on a 32-bit system gets you into trouble, and if you have less, there
will be less of a problem). Filling all of system ram with small tmpfs
files means a single 4K page plus 280 bytes for the minimum inode,
so we need an additional 6MB or 12MB to store the extra timespec
bits. Probably not too bad for a worst-case scenario, but there is
also the case of storing just the inodes but no pages, and that would
be worse.

I've added the linux-arm and linux-mips lists to cc, to see if anyone has
strong opinions on this matter. We don't have to worry about x86-32 here,
because sizeof(struct timespec64) is 12 bytes there anyway, and I don't
think there are any other 32-bit architectures that have large-scale
deployments or additional requirements we don't already have on ARM.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1308584

On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > >
> > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:

Summarizing, here are the open questions that need to be sorted before another
series:
1. What should be part of the series?
	a. y2038 safe data types conversion.
	b. range check and clamping
2. How to achieve a seamless transition?
	Is inode_timespec solution agreed upon to achieve 1a?
	An alternate approach is included in the cover letter.
3. policy for handling out of range timestamps:
	There was no conclusion on this from the previous series as noted in the
	cover letter.
	I think this can be solved by figuring out the answer to question:
	who should have a say in deciding the course of action if the timestamps
	are out of range:
	
	a. sysadmin through sysctl (Arnd's suggestion)
	b. have default vfs handlers with an option for individual fs to override.
	c. clamp and ignore
	d. disable expired fs at compile time (Arnd's suggestion)


> The inode_timespec alias is part of the problem - AFAICT it exists
> because you need a representation that is independent of both the
> old and new in-memory structures.

Maybe you are misunderstanding the fact that only struct inode is changed
to have individual fields. Whereas, timespec64 is used everywhere else in
the series.
inode_timespec is an alias to transition timespec references to
timespec64 without breaking anything. This is needed because we
don't want to change all references to timespec in a single patch like
the cover letter says.
The same RFC Patch 2 has more details.

> The valid timestamp range stuff in the superblock is absolutely
> necessary, but that's something that can be done completely
> independently to the changes for supporting a differnet time storage
> format.

If I'm defining new functions to support new format, then I should at
least get the function signatures right before using them.
These can be in a different patch, but should be in the same patch series,
before they are used anywhere.
For instance,
struct timespec timespec_trunc(struct timespec t, unsigned gran);
should now take superblock as an argument instead of gran.

> And the fs changes cannot really be commented on until the VFs time
> representation is sorted out properly...

Each fs is changed twice in the current approach to transition everything to
timespec64. And, there are different ways of doing this.
For instance, Arnd had an idea different from mine as to how this can be done:
He was suggesting using something like these accessor macros and incorporating
timespec64 from the beginning in the individual filesystems rather than
inode_timespec.
Again, this is independent of how timestamps are stored in struct inode.
There are others that are independent of inode timestamp representation as well


> Besides, have you looked at the existing timestamp definitions? they
> use a struct timespec, which on a 64 bit system:
> 
>         struct timespec            i_atime;              /*    88    16 */
>         struct timespec            i_mtime;              /*   104    16 */
>         struct timespec            i_ctime;              /*   120    16 */
> 
> use 2 longs and are identical to a timespec64 in everything but
> name. These should just be changed to a timespec64, and we suck up
> the fact it increases the size of the 32 bit inode as we have to
> increase it's size to support time > y2038 anyway.
> 
> This is what I meant about premature optimisation - you've got a
> wonderfully complex solution to a problem that we don't need to
> solve to support timestamps >y2038. It's also why it goes down the
> wrong path at this point - most of the changes are not necessary if
> all we need to do is a simple timespec -> timespec64 type change and
> the addition timestamp range limiting in the existing truncation
> function...

The pahole output I pasted in the previous email(on the left) was for
timespec.

Yes, I do know that timespec64 is same as timespec on 64 bit systems:
#if __BITS_PER_LONG == 64
# define timespec64 timespec

I think it's been agreed upon now that inode timestamps will be changed
to use timespec64 as Arnd mentioned, if I do not hear any objections.
The whole purpose of this is to gather comments.

> > The problem really is that
> > there is more than one way of updating these attributes(timestamps in
> > this particular case). The side effect of this is that we don't always
> > call timespec_trunc() before assigning timestamps which can lead to
> > inconsistencies between on disk and in memory inode timestamps.
> 
> That's a problem that can be fixed independently of y2038 support.
> Indeed, we can be quite lazy about updating timestamps - by intent
> and design we usually have different timestamps in memory compared
> to on disk, which is one of the reasons why there are so many
> different ways to change and update timestamps....

This has nothing to do with lazy updates.
This is about writing wrong granularities and non clamped values to
in-memory inode.

-Deepa

[toc] | [prev] | [next] | [standalone]

#1308815

On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> > On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > > >
> > > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> 
> Summarizing, here are the open questions that need to be sorted before another
> series:
> 1. What should be part of the series?
> 	a. y2038 safe data types conversion.
> 	b. range check and clamping

Yes.

> 2. How to achieve a seamless transition?
> 	Is inode_timespec solution agreed upon to achieve 1a?

No. Just convert direct to timespec64.

> 	An alternate approach is included in the cover letter.
> 3. policy for handling out of range timestamps:
> 	There was no conclusion on this from the previous series as noted in the
> 	cover letter.
> 	a. sysadmin through sysctl (Arnd's suggestion)
> 	b. have default vfs handlers with an option for individual fs to override.
> 	c. clamp and ignore

I think it's a mix - if the timestamps come in from userspace,
fail with ERANGE. That could be controlled by sysctl via VFS
part of the ->setattr operation, or in each of the individual FS
implementations. If they come from the kernel (e.g. atime update) then
the generic behvaiour is to warn and continue, filesystems can
otherwise select their own policy for kernel updates via
->update_time.

> 	d. disable expired fs at compile time (Arnd's suggestion)

Not really an option, because it means we can't use filesystems that
interop with other systems (e.g. cameras, etc) because they won't
support y2038k timestamps for a long time, if ever (e.g. vfat).

> > > The problem really is that
> > > there is more than one way of updating these attributes(timestamps in
> > > this particular case). The side effect of this is that we don't always
> > > call timespec_trunc() before assigning timestamps which can lead to
> > > inconsistencies between on disk and in memory inode timestamps.
> > 
> > That's a problem that can be fixed independently of y2038 support.
> > Indeed, we can be quite lazy about updating timestamps - by intent
> > and design we usually have different timestamps in memory compared
> > to on disk, which is one of the reasons why there are so many
> > different ways to change and update timestamps....
> 
> This has nothing to do with lazy updates.
> This is about writing wrong granularities and non clamped values to
> in-memory inode.

Which really shouldn't happen because we should be clamping and/or
truncating timestamps at the creation/entry point into the
VFS/filesystem.

e.g.  current_fs_time(sb) is how filesystems grab the current kernel
time for timestamp updates. Add an equivalent current_fs_time64(sb)
to do return timespec64 and do clamping and limit warning, and now
you have a simple vehicle for converting the VFS and filesystems to
support y2038k clean date formats.

If there are places where filesystems are receiving or using
unchecked timestamps then those are bugs that need fixing. Those
need to be in separate patches to y2038k support...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> > > On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > > > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> >
> > 2. How to achieve a seamless transition?
> > 	Is inode_timespec solution agreed upon to achieve 1a?
> 
> No. Just convert direct to timespec64.

The hard part here is how to split that change into logical patches
per file system. We have already discussed all sorts of ways to
do that, but there is no ideal solution, as you usually end up
either having some really large patches, or you have to modify
the same lines multiple times.

The most promising approaches are:

a) In Deepa's current patch set, some infrastructure is first
   introduced by changing the type from timespec to an identical
   inode_timespec, which lets us convert one file system at a time
   to inode_timespec and then change the type once they are all
   done. The downside is then that all file systems have to get
   touched twice so we end up with timespec64 everywhere.

b) A variation of that which I would do is to use have a smaller
   set of infrastructure first, so we can change one file system
   at a time to timespec64 while leaving the common structures to
   use timespec until all file systems are converted. The downside
   is the use of some conversion macros when accessing the times
   in the inode.
   When the common code is changed, those accessor macros get
   turned into trivial assignments that can be removed up later
   or changed in the same patch.

c) The opposite direction from b) is to first change the common
   code, but then any direct assignment between a timespec in
   a file system and the timespec64 in the inode/iattr/kstat/etc
   first needs a conversion helper so we can build cleanly,
   and then we do one file system at a time to remove them all
   again while changing the internal structures in the
   file system from timespec to timespec64.   

> > 	An alternate approach is included in the cover letter.
> > 3. policy for handling out of range timestamps:
> > 	There was no conclusion on this from the previous series as noted in the
> > 	cover letter.
> > 	a. sysadmin through sysctl (Arnd's suggestion)
> > 	b. have default vfs handlers with an option for individual fs to override.
> > 	c. clamp and ignore
> 
> I think it's a mix - if the timestamps come in from userspace,
> fail with ERANGE. That could be controlled by sysctl via VFS
> part of the ->setattr operation, or in each of the individual FS
> implementations. If they come from the kernel (e.g. atime update) then
> the generic behvaiour is to warn and continue, filesystems can
> otherwise select their own policy for kernel updates via
> ->update_time.

I'd prefer not to have it done by the individual file system
implementation, so we get a consistent behavior. Normally you either
care about correct time stamps, or you care about interoperability
and you don't want to have errors returned here.

It could be done per mount, but that seems overly complicated
for rather little to be gained.

> > 	d. disable expired fs at compile time (Arnd's suggestion)
> 
> Not really an option, because it means we can't use filesystems that
> interop with other systems (e.g. cameras, etc) because they won't
> support y2038k timestamps for a long time, if ever (e.g. vfat).

Let me clarify what my idea is here: I want a global kernel option
that disables all code that has known y2038 issues. If anyone tries
to build an embedded system with support beyond 2038, that should
disable all of those things, including file systems, drivers and
system calls, so we can reasonably assume that everything that works
today with that kernel build will keep working in the future and
not break in random ways.

For a file system, this can be done in a number of ways:

* Most file systems today interpret the time as an unsigned 32-bit
  number (as opposed to signed as ext3, xfs and few others do),
  so as long as we use timespec64 in the syscalls, we are ok.

* Some legacy file systems (maybe hfs) can remain disabled, as
  nobody cares about them any more.

* If we still care about them (e.g. ext2), we can make them support
  only read-only mode. In ext4, this would mean forbidding write
  access to file systems that don't have the extended inode format
  enabled.

Normal users that don't care about not breaking in 2038 obviously
won't set the option, and have the same level of backwards compatibility
support as today.

> > > > The problem really is that
> > > > there is more than one way of updating these attributes(timestamps in
> > > > this particular case). The side effect of this is that we don't always
> > > > call timespec_trunc() before assigning timestamps which can lead to
> > > > inconsistencies between on disk and in memory inode timestamps.
> > > 
> > > That's a problem that can be fixed independently of y2038 support.
> > > Indeed, we can be quite lazy about updating timestamps - by intent
> > > and design we usually have different timestamps in memory compared
> > > to on disk, which is one of the reasons why there are so many
> > > different ways to change and update timestamps....
> > 
> > This has nothing to do with lazy updates.
> > This is about writing wrong granularities and non clamped values to
> > in-memory inode.
> 
> Which really shouldn't happen because we should be clamping and/or
> truncating timestamps at the creation/entry point into the
> VFS/filesystem.
> 
> e.g.  current_fs_time(sb) is how filesystems grab the current kernel
> time for timestamp updates. Add an equivalent current_fs_time64(sb)
> to do return timespec64 and do clamping and limit warning, and now
> you have a simple vehicle for converting the VFS and filesystems to
> support y2038k clean date formats.

I think the current patch series does this already.
 
> If there are places where filesystems are receiving or using
> unchecked timestamps then those are bugs that need fixing. Those
> need to be in separate patches to y2038k support...

Fair enough, but that probably means that patch series will have to
come first. This will also reduce the number of places in which a
separate type conversion function needs to be added.

	Arnd

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote:
> On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> > On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > > On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> > > > On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > > > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > > > > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> > >
> > > 2. How to achieve a seamless transition?
> > > 	Is inode_timespec solution agreed upon to achieve 1a?
> > 
> > No. Just convert direct to timespec64.
> 
> The hard part here is how to split that change into logical patches
> per file system. We have already discussed all sorts of ways to
> do that, but there is no ideal solution, as you usually end up
> either having some really large patches, or you have to modify
> the same lines multiple times.
> 
> The most promising approaches are:
> 
> a) In Deepa's current patch set, some infrastructure is first
>    introduced by changing the type from timespec to an identical
>    inode_timespec, which lets us convert one file system at a time
>    to inode_timespec and then change the type once they are all
>    done. The downside is then that all file systems have to get
>    touched twice so we end up with timespec64 everywhere.
> 
> b) A variation of that which I would do is to use have a smaller
>    set of infrastructure first, so we can change one file system
>    at a time to timespec64 while leaving the common structures to
>    use timespec until all file systems are converted. The downside
>    is the use of some conversion macros when accessing the times
>    in the inode.
>    When the common code is changed, those accessor macros get
>    turned into trivial assignments that can be removed up later
>    or changed in the same patch.
> 
> c) The opposite direction from b) is to first change the common
>    code, but then any direct assignment between a timespec in
>    a file system and the timespec64 in the inode/iattr/kstat/etc
>    first needs a conversion helper so we can build cleanly,
>    and then we do one file system at a time to remove them all
>    again while changing the internal structures in the
>    file system from timespec to timespec64.   

Just a clarification here:
approaches b and c also need some functions that take times as arguments,
including a function pointer in the vfs layer to be supported in both forms:
timespec and timespec64 concurrently.
As included in the cover letter, these are:
generic_update_time(), inode->i_op->update_time(), lease_get_mtime(),
fstack_copy_attr_all(), setattr_copy(), generic_fillattr().

-Deepa

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote:
> On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> > On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > > On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> > > > On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > > > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > > > > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> > >
> > > 2. How to achieve a seamless transition?
> > > 	Is inode_timespec solution agreed upon to achieve 1a?
> > 
> > No. Just convert direct to timespec64.
> 
> The hard part here is how to split that change into logical patches
> per file system. We have already discussed all sorts of ways to
> do that, but there is no ideal solution, as you usually end up
> either having some really large patches, or you have to modify
> the same lines multiple times.
> 
> The most promising approaches are:
> 
> a) In Deepa's current patch set, some infrastructure is first
>    introduced by changing the type from timespec to an identical
>    inode_timespec, which lets us convert one file system at a time
>    to inode_timespec and then change the type once they are all
>    done. The downside is then that all file systems have to get
>    touched twice so we end up with timespec64 everywhere.

Yes, and the result is not pretty.

> b) A variation of that which I would do is to use have a smaller
>    set of infrastructure first, so we can change one file system
>    at a time to timespec64 while leaving the common structures to
>    use timespec until all file systems are converted. The downside
>    is the use of some conversion macros when accessing the times
>    in the inode.
>    When the common code is changed, those accessor macros get
>    turned into trivial assignments that can be removed up later
>    or changed in the same patch.

Doesn't really make a lot of sense to me. We have to change
everything evntually, and it's not that much work to do so up
front...

> c) The opposite direction from b) is to first change the common
>    code, but then any direct assignment between a timespec in
>    a file system and the timespec64 in the inode/iattr/kstat/etc
>    first needs a conversion helper so we can build cleanly,
>    and then we do one file system at a time to remove them all
>    again while changing the internal structures in the
>    file system from timespec to timespec64.   

No new helpers are necessary - we've already got the helper
functions we need. This:

int simple_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
 {
        struct inode *inode = d_inode(old_dentry);
+       struct inode_timespec now = current_fs_time(inode->i_sb);

-       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
+       VFS_INODE_SET_XTIME(i_ctime, inode, now);
+       VFS_INODE_SET_XTIME(i_mtime, dir, now);
+       VFS_INODE_SET_XTIME(i_ctime, dir, now);
        inc_nlink(inode);
.....

is just wrong. All the type conversion and clamping and checking
done in that VFS_INODE_SET_XTIME() should be done in
current_fs_time() and have it return a timespec64 directly. Indeed,
it already does truncation, and can easily be made to do range
clamping, too.  i.e.  the change should simply be:

-       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
+	inode->i_ctime = dir->i_ctime = dir->i_mtime = current_fs_time(inode->i_sb);

This will work cleanly with a s/timespec/timespec64/ API changeover,
too, without the need for any additional helpers at all.

i.e. the first thing to do is make sure everything that
creates/converts timestamps uses/returns a struct timespec that is
clamped and truncated at the earliest possible opportunity. i.e. on
entry to the kernel and when pulled from the kernel time. Once all
the code is using and passing around truncated+clamped struct
timespec (like above), then we can simply do a
s/timespec/timespec64/ on the inode and API, and we're done.

> > I think it's a mix - if the timestamps come in from userspace,
> > fail with ERANGE. That could be controlled by sysctl via VFS
> > part of the ->setattr operation, or in each of the individual FS
> > implementations. If they come from the kernel (e.g. atime update) then
> > the generic behvaiour is to warn and continue, filesystems can
> > otherwise select their own policy for kernel updates via
> > ->update_time.
> 
> I'd prefer not to have it done by the individual file system
> implementation, so we get a consistent behavior.

Can't be helped, because different filesystems have different
timestamp behaviours, and not all use the generic VFS code for
timestamp updates. The filesystems need to use the correct helper
functions to obtain a valid current time, but you can't stop them
from storing and using arbitrary timestamp formats if they so
desire...

> > > 	d. disable expired fs at compile time (Arnd's suggestion)
> > 
> > Not really an option, because it means we can't use filesystems that
> > interop with other systems (e.g. cameras, etc) because they won't
> > support y2038k timestamps for a long time, if ever (e.g. vfat).
> 
> Let me clarify what my idea is here: I want a global kernel option
> that disables all code that has known y2038 issues. If anyone tries
> to build an embedded system with support beyond 2038, that should
> disable all of those things, including file systems, drivers and
> system calls, so we can reasonably assume that everything that works
> today with that kernel build will keep working in the future and
> not break in random ways.

It's not that black and white when it comes to filesystems. y2038k
support is determined by the on-disk structure of the filesystem
being mounted, and that is determined at mount time.  When the
filesystem mounts and sets it's valid timestamp ranges the VFS
will need to decide as to whether the filesystem is allowed to
continue mounting or not.

> For a file system, this can be done in a number of ways:
> 
> * Most file systems today interpret the time as an unsigned 32-bit
>   number (as opposed to signed as ext3, xfs and few others do),
>   so as long as we use timespec64 in the syscalls, we are ok.

Actually, sign conversion is a problem we currently have to be very
careful of.  See, for example, xfstests:tests/generic/258, which
tests timestamps recording times before epoch. i.e. in XFS we have
to convert the unsigned 32 bit disk timestamp to signed 32 bit
before storing it in the VFS timestamp so it behaves correctly on 64
bit systems. This results in us needing to do this when reading the
inode from disk:

+       /*
+        * time is signed, so need to convert to signed 32 bit before
+        * storing in inode timestamp which may be 64 bit. Otherwise
+        * a time before epoch is converted to a time long after epoch
+        * on 64 bit systems.
+        */
+       inode->i_atime.tv_sec = (int)be32_to_cpu(from->di_atime.t_sec);
+       inode->i_atime.tv_nsec = (int)be32_to_cpu(from->di_atime.t_nsec);
+       inode->i_mtime.tv_sec = (int)be32_to_cpu(from->di_mtime.t_sec);
+       inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
+       inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
+       inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
+
(http://oss.sgi.com/archives/xfs/2016-01/msg00456.html)

> * Some legacy file systems (maybe hfs) can remain disabled, as
>   nobody cares about them any more.
> 
> * If we still care about them (e.g. ext2), we can make them support
>   only read-only mode. In ext4, this would mean forbidding write
>   access to file systems that don't have the extended inode format
>   enabled.

For ext2/4, that would have to be handled internally by the
filesystem with feature masks. For other legacy filesystems, then
the VFS mount time checking could allow RO mounts if the supported
ranges are not y2038k clean. Compile time options are not
really the best approach here...

> > If there are places where filesystems are receiving or using
> > unchecked timestamps then those are bugs that need fixing. Those
> > need to be in separate patches to y2038k support...
> 
> Fair enough, but that probably means that patch series will have to
> come first.

Yes, that is normal practice for structuring a non-trivial feature
addition patch series. Bug fixes first, then cleanups, then the
functionality changes.

> This will also reduce the number of places in which a
> separate type conversion function needs to be added.

Precisely. I'm pretty sure this should come down to "no new
conversion functions needed" for the vast majority of the code.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Friday 15 January 2016 08:00:01 Dave Chinner wrote:
> On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote:
> > On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> > > On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > c) The opposite direction from b) is to first change the common
> >    code, but then any direct assignment between a timespec in
> >    a file system and the timespec64 in the inode/iattr/kstat/etc
> >    first needs a conversion helper so we can build cleanly,
> >    and then we do one file system at a time to remove them all
> >    again while changing the internal structures in the
> >    file system from timespec to timespec64.   
> 
> No new helpers are necessary - we've already got the helper
> functions we need. This:
> 
> int simple_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
>  {
>         struct inode *inode = d_inode(old_dentry);
> +       struct inode_timespec now = current_fs_time(inode->i_sb);
> 
> -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> +       VFS_INODE_SET_XTIME(i_ctime, inode, now);
> +       VFS_INODE_SET_XTIME(i_mtime, dir, now);
> +       VFS_INODE_SET_XTIME(i_ctime, dir, now);
>         inc_nlink(inode);
> .....
> 
> is just wrong. All the type conversion and clamping and checking
> done in that VFS_INODE_SET_XTIME() should be done in
> current_fs_time() and have it return a timespec64 directly. Indeed,
> it already does truncation, and can easily be made to do range
> clamping, too.  i.e.  the change should simply be:
>
> -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> +	inode->i_ctime = dir->i_ctime = dir->i_mtime = current_fs_time(inode->i_sb);


Yes, that is the obvious case, and I guess works for at least half the
file systems when they always assign righthand side and lefthand
side of the time stamps using the external types or helpers like
CURRENT_TIME and current_fs_time().

However, there are a couple of file systems that need a bit more refactoring
before we can do this, e.g. in ntfs_truncate:

        if (!IS_NOCMTIME(VFS_I(base_ni)) && !IS_RDONLY(VFS_I(base_ni))) {
                struct timespec now = current_fs_time(VFS_I(base_ni)->i_sb);
                int sync_it = 0;

                if (!timespec_equal(&VFS_I(base_ni)->i_mtime, &now) ||
                    !timespec_equal(&VFS_I(base_ni)->i_ctime, &now))
                        sync_it = 1;
                VFS_I(base_ni)->i_mtime = now;
                VFS_I(base_ni)->i_ctime = now;
	}

The type of the local variable must match the return code of
current_fs_time(), so if we change over i_mtime and current_fs_time
globally, this either has to be rewritten first to avoid the use of
local variables, or it needs temporary conversion helpers, or
it has to be changed in the same patch. None of those is particularly
appealing. There are a few dozen such things in various file systems.

> > > I think it's a mix - if the timestamps come in from userspace,
> > > fail with ERANGE. That could be controlled by sysctl via VFS
> > > part of the ->setattr operation, or in each of the individual FS
> > > implementations. If they come from the kernel (e.g. atime update) then
> > > the generic behvaiour is to warn and continue, filesystems can
> > > otherwise select their own policy for kernel updates via
> > > ->update_time.
> > 
> > I'd prefer not to have it done by the individual file system
> > implementation, so we get a consistent behavior.
> 
> Can't be helped, because different filesystems have different
> timestamp behaviours, and not all use the generic VFS code for
> timestamp updates. The filesystems need to use the correct helper
> functions to obtain a valid current time, but you can't stop them
> from storing and using arbitrary timestamp formats if they so
> desire...

I mean the decision whether to clamp or error on an overflow
should be done consistently. Having a global sysctl knob or
a compile-time option is better than having each file system
implementor take a guess at what users might prefer, if we can't
come up with a behavior (e.g. clamp all the time, or error out
all the time) that everybody agrees is always correct.

> > > > 	d. disable expired fs at compile time (Arnd's suggestion)
> > > 
> > > Not really an option, because it means we can't use filesystems that
> > > interop with other systems (e.g. cameras, etc) because they won't
> > > support y2038k timestamps for a long time, if ever (e.g. vfat).
> > 
> > Let me clarify what my idea is here: I want a global kernel option
> > that disables all code that has known y2038 issues. If anyone tries
> > to build an embedded system with support beyond 2038, that should
> > disable all of those things, including file systems, drivers and
> > system calls, so we can reasonably assume that everything that works
> > today with that kernel build will keep working in the future and
> > not break in random ways.
> 
> It's not that black and white when it comes to filesystems. y2038k
> support is determined by the on-disk structure of the filesystem
> being mounted, and that is determined at mount time.  When the
> filesystem mounts and sets it's valid timestamp ranges the VFS
> will need to decide as to whether the filesystem is allowed to
> continue mounting or not.

Some file systems are always broken around 2038 (e.g. HFS in 2040),
so if we can't fix them, I want to be able to turn them off
in Kconfig along with the 32-bit time_t syscalls. For those
where y2038 support depends on on-disk feature flags (ext4 and
xfs I guess, maybe one or two more), the config option can
turn off write support for the old format.

This is a rather important part of the y2038 work: If anyone cares about
y2038 problems in this decade, they want to deploy systems with extremely
long service contracts and don't want to update them 20 years from now
to fix an obscure bug that could be prevented today, so we try hard to
identify every line of code that won't work then as it does today.

> > For a file system, this can be done in a number of ways:
> > 
> > * Most file systems today interpret the time as an unsigned 32-bit
> >   number (as opposed to signed as ext3, xfs and few others do),
> >   so as long as we use timespec64 in the syscalls, we are ok.
> 
> Actually, sign conversion is a problem we currently have to be very
> careful of.  See, for example, xfstests:tests/generic/258, which
> tests timestamps recording times before epoch. i.e. in XFS we have
> to convert the unsigned 32 bit disk timestamp to signed 32 bit
> before storing it in the VFS timestamp so it behaves correctly on 64
> bit systems. This results in us needing to do this when reading the
> inode from disk:
> 
> +       /*
> +        * time is signed, so need to convert to signed 32 bit before
> +        * storing in inode timestamp which may be 64 bit. Otherwise
> +        * a time before epoch is converted to a time long after epoch
> +        * on 64 bit systems.
> +        */
> +       inode->i_atime.tv_sec = (int)be32_to_cpu(from->di_atime.t_sec);
> +       inode->i_atime.tv_nsec = (int)be32_to_cpu(from->di_atime.t_nsec);
> +       inode->i_mtime.tv_sec = (int)be32_to_cpu(from->di_mtime.t_sec);
> +       inode->i_mtime.tv_nsec = (int)be32_to_cpu(from->di_mtime.t_nsec);
> +       inode->i_ctime.tv_sec = (int)be32_to_cpu(from->di_ctime.t_sec);
> +       inode->i_ctime.tv_nsec = (int)be32_to_cpu(from->di_ctime.t_nsec);
> +
> (http://oss.sgi.com/archives/xfs/2016-01/msg00456.html)

I'm very aware of this issue. Most file system developers however were
not, so e.g. a timestamp on btrfs is interpreted differently on 32-bit
and 64-bit kernels. Some file systems (AFS, NFSv3?) explicitly define
the timestamps as unsigned, and most others that store 32-bit seconds
apparently never thought about the issue and happen to use unsigned
interpretation on 64-bit systems, since that is what you get out of
be32_to_cpu() without adding the cast.

For the y2038 case, this means we are lucky: almost all the users today
have 64-bit hardware, so they can already represent timestamps in the
range from 1970 to 2106. Once we have 64-bit timestamps in 32-bit user
space, we just need to make that use the same format we use on the
64-bit machines already.

ext2/3/4, xfs and ocfs2 (maybe one or two more, I'd have to check)
currently behave in a consistent manner across 32-bit and 64-bit
architectures by allowing a range between 1902 and 2037, and we
obviously don't have a choice there but to keep that current
behavior, and extend the time format in one way or another to
store additional bits for the epoch.

> > * Some legacy file systems (maybe hfs) can remain disabled, as
> >   nobody cares about them any more.
> > 
> > * If we still care about them (e.g. ext2), we can make them support
> >   only read-only mode. In ext4, this would mean forbidding write
> >   access to file systems that don't have the extended inode format
> >   enabled.
> 
> For ext2/4, that would have to be handled internally by the
> filesystem with feature masks. For other legacy filesystems, then
> the VFS mount time checking could allow RO mounts if the supported
> ranges are not y2038k clean. Compile time options are not
> really the best approach here...

I'm not following the line of thought here. We have some users
that want ext4 to mount old file system images without long
inodes writable, because they don't care about the 2038 problem.
We also have other users that want to force the same file system
image to be read-only because they want to ensure that it does
not stop working correctly when the time overflow happens while
the fs is mounted.

If you don't want a compile-time option for it, how do you suggest
we decide which case we have?

	Arnd

[toc] | [prev] | [next] | [standalone]

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

On Thursday 14 January 2016 23:46:16 Arnd Bergmann wrote:
> 
> I'm not following the line of thought here. We have some users
> that want ext4 to mount old file system images without long
> inodes writable, because they don't care about the 2038 problem.
> We also have other users that want to force the same file system
> image to be read-only because they want to ensure that it does
> not stop working correctly when the time overflow happens while
> the fs is mounted.
> 
> If you don't want a compile-time option for it, how do you suggest
> we decide which case we have?

In case that came across wrong, I'm assuming that the first
user also wants all the system calls enabled that pass 32-bit
time_t values, while the second one wants them all left out from
the kernel to ensure that no user space program gets incorrect
data. This could be done using a sysctl of course, but I still
think we want a compile-time option for the syscalls for clarity,
and I would simply use the same compile-time option to determine
the behavior of the file system, network protocols and device
drivers that deal with 32-bit timestamps outside of the kernel.

	Arnd

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jan 14, 2016 at 11:54:36PM +0100, Arnd Bergmann wrote:
> On Thursday 14 January 2016 23:46:16 Arnd Bergmann wrote:
> > 
> > I'm not following the line of thought here. We have some users
> > that want ext4 to mount old file system images without long
> > inodes writable, because they don't care about the 2038 problem.
> > We also have other users that want to force the same file system
> > image to be read-only because they want to ensure that it does
> > not stop working correctly when the time overflow happens while
> > the fs is mounted.
> > 
> > If you don't want a compile-time option for it, how do you suggest
> > we decide which case we have?
> 
> In case that came across wrong, I'm assuming that the first
> user also wants all the system calls enabled that pass 32-bit
> time_t values, while the second one wants them all left out from
> the kernel to ensure that no user space program gets incorrect
> data.

system call API support is a completely different class of problem.
It's out of the scope of this patchset, and really I don't care what
you do with them.

The point I'm making is that we'll have to modify all the existing
filesystem code to supply a valid timestamp range to the VFS at
mount time for the range checking/clamping, similar to how we do the
granularity specification right now. That means we can do rejection
of non-y2038k compliant filesystems at runtime based on what the
filesystem tells the VFS it supports..  Set up the default to be
reject if rw, allow if ro, and provide a mount option to override ad
allow mounting rw.

Users can then make the decision when mounting their filesystems.
If they are system/automatically mounted filesystems and aren't
y2038k compliant, then the override option can be added to
/etc/fstab and we're all good. If the truly paranoid users want to
disallow the override and/or read only mount options, then add a
sysctl to control that.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jan 14, 2016 at 11:46:16PM +0100, Arnd Bergmann wrote:
> On Friday 15 January 2016 08:00:01 Dave Chinner wrote:
> > On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote:
> > > On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> > > > On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > > c) The opposite direction from b) is to first change the common
> > >    code, but then any direct assignment between a timespec in
> > >    a file system and the timespec64 in the inode/iattr/kstat/etc
> > >    first needs a conversion helper so we can build cleanly,
> > >    and then we do one file system at a time to remove them all
> > >    again while changing the internal structures in the
> > >    file system from timespec to timespec64.   
> > 
> > No new helpers are necessary - we've already got the helper
> > functions we need. This:
> > 
> > int simple_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
> >  {
> >         struct inode *inode = d_inode(old_dentry);
> > +       struct inode_timespec now = current_fs_time(inode->i_sb);
> > 
> > -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> > +       VFS_INODE_SET_XTIME(i_ctime, inode, now);
> > +       VFS_INODE_SET_XTIME(i_mtime, dir, now);
> > +       VFS_INODE_SET_XTIME(i_ctime, dir, now);
> >         inc_nlink(inode);
> > .....
> > 
> > is just wrong. All the type conversion and clamping and checking
> > done in that VFS_INODE_SET_XTIME() should be done in
> > current_fs_time() and have it return a timespec64 directly. Indeed,
> > it already does truncation, and can easily be made to do range
> > clamping, too.  i.e.  the change should simply be:
> >
> > -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> > +	inode->i_ctime = dir->i_ctime = dir->i_mtime = current_fs_time(inode->i_sb);
> 
> 
> Yes, that is the obvious case, and I guess works for at least half the
> file systems when they always assign righthand side and lefthand
> side of the time stamps using the external types or helpers like
> CURRENT_TIME and current_fs_time().
> 
> However, there are a couple of file systems that need a bit more refactoring
> before we can do this, e.g. in ntfs_truncate:

Sure, and nfs is a pain because of all it's internal use of
timespecs, too. But we have these timespec_to_timespec64 helper
functions, and that's what we should use in these cases where the
filesystem cannot support full 64 bit timestamps internally. In
those cases, they'll be telling the superblock this at mount time
things like current_fs_time() won't be returning then a timestamp
that is out of range for a 32 bit timestamp....

>         if (!IS_NOCMTIME(VFS_I(base_ni)) && !IS_RDONLY(VFS_I(base_ni))) {
>                 struct timespec now = current_fs_time(VFS_I(base_ni)->i_sb);
>                 int sync_it = 0;
> 
>                 if (!timespec_equal(&VFS_I(base_ni)->i_mtime, &now) ||
>                     !timespec_equal(&VFS_I(base_ni)->i_ctime, &now))
>                         sync_it = 1;
>                 VFS_I(base_ni)->i_mtime = now;
>                 VFS_I(base_ni)->i_ctime = now;
> 	}
> 
> The type of the local variable must match the return code of
> current_fs_time(), so if we change over i_mtime and current_fs_time
> globally, this either has to be rewritten first to avoid the use of
> local variables, or it needs temporary conversion helpers, or
> it has to be changed in the same patch. None of those is particularly
> appealing. There are a few dozen such things in various file systems.

it gets rewritten to:

		struct timespec now;

		now = timespec64_to_timespec(current_fs_time(VFS_I(base_ni)->i_sb));
....

and the valid timestamp range for ntfs is set to 32bit timestamps.
This then leaves it up to the filesystem developers to make
the ntfs filesystem code 64 bit timestamp clean if the on disk
format is ever changed to support 64 bit times.

Same goes for NFS, and any of the other filesystems that use struct
timespec internally for time representation.

> > > > I think it's a mix - if the timestamps come in from userspace,
> > > > fail with ERANGE. That could be controlled by sysctl via VFS
> > > > part of the ->setattr operation, or in each of the individual FS
> > > > implementations. If they come from the kernel (e.g. atime update) then
> > > > the generic behvaiour is to warn and continue, filesystems can
> > > > otherwise select their own policy for kernel updates via
> > > > ->update_time.
> > > 
> > > I'd prefer not to have it done by the individual file system
> > > implementation, so we get a consistent behavior.
> > 
> > Can't be helped, because different filesystems have different
> > timestamp behaviours, and not all use the generic VFS code for
> > timestamp updates. The filesystems need to use the correct helper
> > functions to obtain a valid current time, but you can't stop them
> > from storing and using arbitrary timestamp formats if they so
> > desire...
> 
> I mean the decision whether to clamp or error on an overflow
> should be done consistently.

Sure, but that comes from using the helpers we already have, and
applying the clamping at a point where errors can be returned to
userspace. current_fs_time() should never return a timestamp outside
what the filesystem has said it supports and we've validated that
behaviour at mount time. hence it's only user provided timestamps
that we need range errors on.

> Having a global sysctl knob or
> a compile-time option is better than having each file system
> implementor take a guess at what users might prefer, if we can't
> come up with a behavior (e.g. clamp all the time, or error out
> all the time) that everybody agrees is always correct.

filesystem implementors will use the helper funtions that are
provided for this. If they don't (like all the current use of
CURRENT_TIME), then that's a bug that needs fixing. i.e. we need
a timespec_clamp() function, similar to timespec_trunc(), and y2038k
compliant filesystems and syscalls need to use them....

> > > > 
> > > > Not really an option, because it means we can't use filesystems that
> > > > interop with other systems (e.g. cameras, etc) because they won't
> > > > support y2038k timestamps for a long time, if ever (e.g. vfat).
> > > 
> > > Let me clarify what my idea is here: I want a global kernel option
> > > that disables all code that has known y2038 issues. If anyone tries
> > > to build an embedded system with support beyond 2038, that should
> > > disable all of those things, including file systems, drivers and
> > > system calls, so we can reasonably assume that everything that works
> > > today with that kernel build will keep working in the future and
> > > not break in random ways.
> > 
> > It's not that black and white when it comes to filesystems. y2038k
> > support is determined by the on-disk structure of the filesystem
> > being mounted, and that is determined at mount time.  When the
> > filesystem mounts and sets it's valid timestamp ranges the VFS
> > will need to decide as to whether the filesystem is allowed to
> > continue mounting or not.
> 
> Some file systems are always broken around 2038 (e.g. HFS in 2040),
> so if we can't fix them, I want to be able to turn them off
> in Kconfig along with the 32-bit time_t syscalls.

That can be done with kconfig depends rules - it has nothing to do
with this patch set.

> ext2/3/4, xfs and ocfs2 (maybe one or two more, I'd have to check)
> currently behave in a consistent manner across 32-bit and 64-bit
> architectures by allowing a range between 1902 and 2037, and we
> obviously don't have a choice there but to keep that current
> behavior, and extend the time format in one way or another to
> store additional bits for the epoch.

That's a filesystem implementation problem, not a generic inode
timestamp problem. i.e. this is handled when the filesystem converts
the inode timestamp from a timespec64 in the struct inode to
whatever format it stores the timestamp on disk. That conversion
does not change just because the VFS inode moves from a timespec to
a timespec64. Again, those on-disk format changes to support beyond
the current epoch are outside the scope of this patchset, because
they are not affected by the timestamp format the VFS choses to use.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

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

On Fri, Jan 15, 2016 at 08:00:01AM +1100, Dave Chinner wrote:
> On Thu, Jan 14, 2016 at 05:53:21PM +0100, Arnd Bergmann wrote:
> > On Thursday 14 January 2016 08:04:36 Dave Chinner wrote:
> > > On Wed, Jan 13, 2016 at 08:33:16AM -0800, Deepa Dinamani wrote:
> > > > On Tue, Jan 12, 2016 at 07:29:57PM +1100, Dave Chinner wrote:
> > > > > On Mon, Jan 11, 2016 at 09:42:36PM -0800, Deepa Dinamani wrote:
> > > > > > > On Jan 11, 2016, at 04:33, Dave Chinner <david@fromorbit.com> wrote:
> > > > > > >> On Wed, Jan 06, 2016 at 09:35:59PM -0800, Deepa Dinamani wrote:
> > > >
> > c) The opposite direction from b) is to first change the common
> >    code, but then any direct assignment between a timespec in
> >    a file system and the timespec64 in the inode/iattr/kstat/etc
> >    first needs a conversion helper so we can build cleanly,
> >    and then we do one file system at a time to remove them all
> >    again while changing the internal structures in the
> >    file system from timespec to timespec64.   
> 
> No new helpers are necessary - we've already got the helper
> functions we need. This:
> 
> int simple_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
>  {
>         struct inode *inode = d_inode(old_dentry);
> +       struct inode_timespec now = current_fs_time(inode->i_sb);
> 
> -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> +       VFS_INODE_SET_XTIME(i_ctime, inode, now);
> +       VFS_INODE_SET_XTIME(i_mtime, dir, now);
> +       VFS_INODE_SET_XTIME(i_ctime, dir, now);
>         inc_nlink(inode);
> .....
> 
> is just wrong. All the type conversion and clamping and checking
> done in that VFS_INODE_SET_XTIME() should be done in
> current_fs_time() and have it return a timespec64 directly. Indeed,
> it already does truncation, and can easily be made to do range
> clamping, too.  i.e.  the change should simply be:
> 
> -       inode->i_ctime = dir->i_ctime = dir->i_mtime = CURRENT_TIME;
> +	inode->i_ctime = dir->i_ctime = dir->i_mtime = current_fs_time(inode->i_sb);

The current patch series already does this and macros don't do any clamping.
The accesors are only illustrating a way of using a callback when clamping is detected.
And, if we don't need accessors, I will find a different way of doing that if we agree
it is necessary.

+struct timespec64 current_fs_time(struct super_block *sb)
+{
+       struct timespec64 now = current_kernel_time64();
+
+       return fs_time_trunc(now, sb);
+}
+EXPORT_SYMBOL(current_fs_time);
+
+struct timespec64 current_fs_time_sec(struct super_block *sb)
+{
+       struct timespec64 ts = {ktime_get_real_seconds(), 0};
+
+       /* range check for time. */
+       fs_time_range_check(sb, &ts);
+
+       return ts;
+}

+struct inode_timespec
+fs_time_trunc(struct inode_timespec t, struct super_block *sb)
+{
+       u32 gran = sb->s_time_gran;
+
+       /* range check for time. */
+       fs_time_range_check(sb, &t);
+       if (unlikely(is_fs_timestamp_bad(t)))
+               return t;
+
+       /* Avoid division in the common cases 1 ns and 1 s. */
+       if (gran == 1)
+               ;/* nothing */
+       else if (gran == NSEC_PER_SEC)
+               t.tv_nsec = 0;
+       else if (gran > 1 && gran < NSEC_PER_SEC)
+               t.tv_nsec -= t.tv_nsec % gran;
+       else
+               WARN(1, "illegal file time granularity: %u", gran);
+
+       return t;
+}

But really, you are still misunderstanding what inode_timespec does.
It only introduces aliases and not the accessors.
This is only a way to avoid code duplication since we cannot change all fs
at one time.

And, this is what you would do if you were coding in any object oriented
language. This is object polymorphism.

So, I will paste here again. Whatever is below is all the extra code
inode_timespec introduces:

+#ifdef CONFIG_FS_USES_64BIT_TIME
+
+/* Place holder defines until CONFIG_FS_USES_64BIT_TIME
+ * is enabled.
+ * timespec64 data type and functions will be used at that
+ * time directly and these defines will be deleted.
+ */
+#define inode_timespec timespec64
+
+#define inode_timespec_compare timespec64_compare
+#define inode_timespec_equal   timespec64_equal
+
+#else
+
+#define inode_timespec timespec
+
+#define inode_timespec_compare timespec_compare
+#define inode_timespec_equal   timespec_equal
+
+#endif
+

-Deepa

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web