linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Deepa Dinamani <deepa.kernel@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [GIT PULL] y2038 changes for vfs
Date: Tue, 24 May 2016 17:10:20 -0700	[thread overview]
Message-ID: <CABeXuvp1Q24rFHbJaO4CHdDJ8cGOgexkGZEQ-2cDqZJBwmLE7w@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFyUhv1qQMkON1S+_trQ+4iwd2S4Ty8BHe=g-SPLTscasg@mail.gmail.com>

On Tue, May 24, 2016 at 3:44 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, May 24, 2016 at 3:23 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> 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);

If we use current_fs_time() instead of CURRENT_TIME_SEC, then it also
means that all the filesystems now using these should use correct
granularity or provide update_time callbacks for time. This is not
true today. Eg: fat uses CURRENT_TIME_SEC everywhere but uses
generic_update_time() and so the granularity is in nanoseconds for in
memory timestamps even though this was not intended.

> Actually, looking at the users, most of them don't have the superblock
> directly as a variable, so it might be better to just make
> current_fs_time() take the inode pointer instead.

This is a much bigger change as current_fs_time() is used in many
places for timestamps where inode is not part of the functions.
And, this would mean reordering code. Eg:  cifs_create_dfs_fattr(),
btrfs_update_root_times()
And, we are really only relying on super_block attributes in the function.

> That would make the conversion simpler, and it can then do the
> inode->i_sb thing when it is converted to actually take the filesystem
> limits and time granularity into account.
>
> I suspect you could do 95% with a fairly simply coccinelle script. Or
> even just use 'sed', with something like
>
>   sed 's/\([a-z]*\)->i_\([amc]\)time = CURRENT_TIME_SEC/\1->i_\2time =
> current_fs_time(\1)/'
>
> seems to get close.
>
> Run that over the tree, fix up the few cases it doesn't catch, remove
> the broken CURRENT_TIME_SEC thing, and voilá, you're pretty much done.
>
> No?

Yes, replacing CURRENT_TIME_SEC with current_fs_time_sec() is pretty
straight forward.
The reason this was split into many patches was because we thought it
would be easier for individual maintainers to review the code.
This is particularly true for filesystem ranges.
For instance, CIFS has multiple ranges based on the version used: cifs
modern or cifs smb.

But, it should be fine as a single patch as well.

-Deepa

  reply	other threads:[~2016-05-25  0:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24 20:11 [GIT PULL] y2038 changes for vfs Arnd Bergmann
2016-05-24 22:23 ` Linus Torvalds
2016-05-24 22:44   ` Linus Torvalds
2016-05-25  0:10     ` Deepa Dinamani [this message]
2016-05-25 16:03   ` Arnd Bergmann
2016-05-25 21:33     ` Dave Chinner

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=CABeXuvp1Q24rFHbJaO4CHdDJ8cGOgexkGZEQ-2cDqZJBwmLE7w@mail.gmail.com \
    --to=deepa.kernel@gmail.com \
    --cc=arnd@arndb.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).