linux-kernel - Re: [GIT PULL] y2038 changes for vfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFz+Vys=S-K3f4hY0RumUx2TJSivzWCCAOtNPxU+uv7KQw@mail.gmail.com>
Date:	Tue, 24 May 2016 15:23:39 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux FS Devel <linux-fsdevel@...r.kernel.org>,
	Deepa Dinamani <deepa.kernel@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [GIT PULL] y2038 changes for vfs

On Tue, May 24, 2016 at 1:11 PM, Arnd Bergmann <arnd@...db.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