Re: [GIT PULL] y2038 changes for vfs

From: Deepa Dinamani
Date: Tue May 24 2016 - 20:10:26 EST


On Tue, May 24, 2016 at 3:44 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, May 24, 2016 at 3:23 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> 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