All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: y2038@lists.linaro.org
Cc: Dave Chinner <david@fromorbit.com>,
	Deepa Dinamani <deepa.kernel@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Y2038] [RFC 02/15] vfs: Change all structures to support 64 bit time
Date: Thu, 14 Jan 2016 17:53:21 +0100	[thread overview]
Message-ID: <5579638.biZvj0xqhY@wuerfel> (raw)
In-Reply-To: <20160113210436.GR10456@dastard>

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

  reply	other threads:[~2016-01-14 16:53 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07  5:35 [RFC 00/15] Add 64 bit timestamp support Deepa Dinamani
2016-01-07  5:35 ` [RFC 01/15] fs: add Kconfig entry CONFIG_FS_USES_64BIT_TIME Deepa Dinamani
2016-01-07  5:35 ` [RFC 02/15] vfs: Change all structures to support 64 bit time Deepa Dinamani
2016-01-10 23:03   ` Dave Chinner
2016-01-12  5:42     ` Deepa Dinamani
2016-01-12  8:29       ` Dave Chinner
2016-01-12  9:27         ` [Y2038] " Arnd Bergmann
2016-01-13  6:27           ` Dave Chinner
2016-01-13  9:20             ` Arnd Bergmann
2016-01-13  9:20               ` Arnd Bergmann
2016-01-13 16:33         ` Deepa Dinamani
2016-01-13 21:04           ` Dave Chinner
2016-01-14 16:53             ` Arnd Bergmann [this message]
2016-01-14 18:00               ` [Y2038] " Deepa Dinamani
2016-01-14 21:00               ` Dave Chinner
2016-01-14 22:46                 ` Arnd Bergmann
2016-01-14 22:54                   ` Arnd Bergmann
2016-01-15  2:27                     ` Dave Chinner
2016-01-15 17:01                       ` Arnd Bergmann
2016-01-15 22:41                         ` Dave Chinner
2016-01-15  2:49                   ` Dave Chinner
2016-01-15 16:50                     ` Arnd Bergmann
2016-01-16 19:14                       ` Andreas Dilger
2016-01-16 23:36                         ` Arnd Bergmann
2016-01-17  2:30                           ` Andreas Dilger
2016-01-18  6:09                             ` Deepa Dinamani
2016-01-18 10:56                               ` Arnd Bergmann
2016-01-18 17:40                                 ` Deepa Dinamani
2016-01-18 19:53                                   ` Arnd Bergmann
2016-01-18 21:14                                     ` Dave Chinner
2016-01-18 21:46                                       ` Arnd Bergmann
2016-01-19  1:38                                         ` Dave Chinner
2016-01-19  5:27                                           ` Deepa Dinamani
2016-01-19 20:49                                             ` Dave Chinner
2016-01-19 22:25                                               ` Arnd Bergmann
2016-01-20  5:12                                                 ` Deepa Dinamani
2016-01-20 15:04                                                   ` Deepa Dinamani
2016-01-20 23:06                                                 ` Dave Chinner
2016-01-20 23:17                                                   ` [Y2038] " Arnd Bergmann
2016-01-27  6:26                                                     ` Deepa Dinamani
2016-01-15  5:03                 ` Deepa Dinamani
2016-01-07  5:36 ` [RFC 03/15] kernel: time: Add macros and functions " Deepa Dinamani
2016-01-07  5:36 ` [RFC 04/15] vfs: Add support for vfs code to use " Deepa Dinamani
2016-01-07  5:36 ` [RFC 05/15] fs: cifs: Add support for cifs " Deepa Dinamani
2016-01-07  5:36 ` [RFC 06/15] fs: fat: convert fat to " Deepa Dinamani
2016-01-07  5:36 ` [RFC 07/15] fs: ext4: convert to use " Deepa Dinamani
2016-01-07  5:36 ` [RFC 08/15] fs: Enable " Deepa Dinamani
2016-01-07  5:36 ` [RFC 09/15] fs: cifs: replace inode_timespec with timespec64 Deepa Dinamani
2016-01-07  5:36 ` [RFC 10/15] fs: fat: " Deepa Dinamani
2016-01-07  5:36 ` [RFC 11/15] fs: ext4: " Deepa Dinamani
2016-01-07  5:36 ` [RFC 12/15] vfs: remove inode_timespec and timespec references Deepa Dinamani
2016-01-07  5:36 ` [RFC 13/15] kernel: time: change inode_timespec to timespec64 Deepa Dinamani
2016-01-07  8:50   ` Michael Adam
2016-01-07 10:42     ` Deepa Dinamani
2016-01-07  5:36 ` [RFC 14/15] vfs: Remove inode_timespec aliases Deepa Dinamani
2016-01-07  5:36 ` [RFC 15/15] fs: Drop CONFIG_FS_USES_64BIT_TIME Deepa Dinamani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5579638.biZvj0xqhY@wuerfel \
    --to=arnd@arndb.de \
    --cc=david@fromorbit.com \
    --cc=deepa.kernel@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=y2038@lists.linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.