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

Groups > linux.kernel > #1406472

Re: [GIT PULL] y2038 changes for vfs

Show key headers only | View raw

On Tue, May 24, 2016 at 1:11 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The following changes since commit bf16200689118d19de1b8d2a3c314fc21f5dc7bb:
>
>   Linux 4.6-rc3 (2016-04-10 17:58:30 -0700)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git tags/y2038-4.7

The more I look at this, the less I like it.

There doesn't even seem to be any *point* to the preparatory patches.
I'm not seeing what any of those patches actually help prepare. The
two new superblock fields that it adds, for example, should likely
never be touched directly by any code in the first place, so adding
them only encourages people to add more "preparatory" patches to
filesystems that simply don't seem sensible. It's not clear we want a
seconds-based interface there, when in many ways ktime_t is much
*much* preferable for internal kernel representations for the next
hundred years or so.

For example, preparing to replace CURRENT_TIME_SEC with
current_fs_time_sec() is going to be a huge big patch replacing every
single user *anyway* due to the addition of the superblock parameter.
And since we'd have to change the type in the inode, that will be a
flag-day anyway.

So I'm not seeing real advantages to the prep-work. What does it
actually *help*? I'd have seen more point to it if it had actually
converted all the existing CURRENT_TIME_SEC cases, and basically said
"the code is now syntactically ready to start using per-sb limits".

I don't much see the point of a preparatory patch that just paves the
way for a hundred other small pointless one-liner patches, when it
shouldn't be a problem to just do it in one go.

Just as an example: code that does

        dir->i_mtime = dir->i_ctime = CURRENT_TIME_SEC;

could pretty mechanically be converted to

        dir->i_mtime = dir->i_ctime = current_fs_time(sb);

and there really is only about a hundred of those. THAT would be a
preparatory patch that actually adds value.

IOW, do it as one single patch that gets rid of a bad interface, not
as "one pointless preparatory patch that than makes it possible to
make a hundred other pointless patches to use it".

It's not like it's hard to compile-test the pretty mechanical
conversion. There are no architecture-specific users, so I suspect
that a trivial "make allmodconfig" build will catch all the cases.

Why drag something like this out, in other words?

                  Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[GIT PULL] y2038 changes for vfs Arnd Bergmann <arnd@arndb.de> - 2016-05-24 22:20 +0200
  Re: [GIT PULL] y2038 changes for vfs Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-25 00:30 +0200
    Re: [GIT PULL] y2038 changes for vfs Linus Torvalds <torvalds@linux-foundation.org> - 2016-05-25 00:50 +0200
      Re: [GIT PULL] y2038 changes for vfs Deepa Dinamani <deepa.kernel@gmail.com> - 2016-05-25 02:20 +0200
    Re: [GIT PULL] y2038 changes for vfs Arnd Bergmann <arnd@arndb.de> - 2016-05-25 18:10 +0200
      Re: [GIT PULL] y2038 changes for vfs Dave Chinner <david@fromorbit.com> - 2016-05-25 23:40 +0200

csiph-web