All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: y2038@lists.linaro.org, 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: Wed, 13 Jan 2016 17:27:16 +1100	[thread overview]
Message-ID: <20160113062716.GJ6033@dastard> (raw)
In-Reply-To: <4788428.RCG4WOvRdT@wuerfel>

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

  reply	other threads:[~2016-01-13  6:27 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 [this message]
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             ` [Y2038] " Arnd Bergmann
2016-01-14 18:00               ` 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=20160113062716.GJ6033@dastard \
    --to=david@fromorbit.com \
    --cc=arnd@arndb.de \
    --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.