https://lore.kernel.org/linux-fsdevel/20190818165817.32634-5-deepa.kernel@gmail.
com/
Running only a subset of the LTP testsuite on today's linux-next with the above
commit is now generating ~800 warnings on this machine which seems a bit crazy.
[ 2130.970782] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#40961: comm statx04: inode does not support timestamps beyond 2038
[ 2130.970808] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#40961: comm statx04: inode does not support timestamps beyond 2038
[ 2130.970838] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#40961: comm statx04: inode does not support timestamps beyond 2038
[ 2130.971440] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#40961: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847613] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847647] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847681] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847717] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847774] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847817] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847909] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.847970] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.848004] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2131.848415] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#32769: comm statx04: inode does not support timestamps beyond 2038
[ 2134.753752] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.753783] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.753814] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.753847] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.753889] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.753929] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.754021] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.754064] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
#12: comm statx05: inode does not support timestamps beyond 2038
[ 2134.754105] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
#12: comm statx05: inode does not support timestamps beyond 2038
Actually this warning is coming from this patch:
https://lore.kernel.org/linux-fsdevel/[email protected]/
([PATCH v8 09/20] ext4: Initialize timestamps limits).
This is the code generating the warning:
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9c7f4036021b..ae5d0c86aba2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -832,11 +832,15 @@ static inline void
ext4_decode_extra_time(struct timespec64 *time,
#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)
\
do {
\
- (raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);
\
if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ##
_extra)) {\
+ (raw_inode)->xtime =
cpu_to_le32((inode)->xtime.tv_sec); \
(raw_inode)->xtime ## _extra =
\
ext4_encode_extra_time(&(inode)->xtime); \
}
\
+ else {\
+ (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t,
(inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
+ ext4_warning_inode(inode, "inode does not support
timestamps beyond 2038"); \
+ } \
} while (0)
This prints a warning for each inode that doesn't extend limits beyond
2038. It is rate limited by the ext4_warning_inode().
Looks like your filesystem has inodes that cannot be extended.
We could use a different rate limit or ignore this corner case. Do the
maintainers have a preference?
-Deepa
On Tue, Sep 3, 2019 at 8:18 AM Qian Cai <[email protected]> wrote:
>
> https://lore.kernel.org/linux-fsdevel/20190818165817.32634-5-deepa.kernel@gmail.
> com/
>
> Running only a subset of the LTP testsuite on today's linux-next with the above
> commit is now generating ~800 warnings on this machine which seems a bit crazy.
>
> [ 2130.970782] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #40961: comm statx04: inode does not support timestamps beyond 2038
> [ 2130.970808] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #40961: comm statx04: inode does not support timestamps beyond 2038
> [ 2130.970838] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #40961: comm statx04: inode does not support timestamps beyond 2038
> [ 2130.971440] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #40961: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847613] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847647] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847681] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847717] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847774] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847817] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847909] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.847970] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.848004] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2131.848415] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #32769: comm statx04: inode does not support timestamps beyond 2038
> [ 2134.753752] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.753783] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.753814] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.753847] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.753889] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.753929] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.754021] EXT4-fs warning (device loop0): ext4_do_update_inode:5261: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.754064] EXT4-fs warning (device loop0): ext4_do_update_inode:5262: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
> [ 2134.754105] EXT4-fs warning (device loop0): ext4_do_update_inode:5263: inode
> #12: comm statx05: inode does not support timestamps beyond 2038
We might also want to consider updating the file system the LTP is
being run on here.
-Deepa
On Tue, 2019-09-03 at 09:36 -0700, Deepa Dinamani wrote:
> We might also want to consider updating the file system the LTP is
> being run on here.
It simply format (mkfs.ext4) a loop back device on ext4 with the kernel.
CONFIG_EXT4_FS=m
# CONFIG_EXT4_USE_FOR_EXT2 is not set
# CONFIG_EXT4_FS_POSIX_ACL is not set
# CONFIG_EXT4_FS_SECURITY is not set
# CONFIG_EXT4_DEBUG is not set
using e2fsprogs-1.44.6. Do you mean people now need to update the kernel to
enable additional config to avoid the spam of warnings now?
On Sep 3, 2019, at 12:15 PM, Qian Cai <[email protected]> wrote:
>
> On Tue, 2019-09-03 at 09:36 -0700, Deepa Dinamani wrote:
>> We might also want to consider updating the file system the LTP is
>> being run on here.
>
> It simply format (mkfs.ext4) a loop back device on ext4 with the kernel.
>
> CONFIG_EXT4_FS=m
> # CONFIG_EXT4_USE_FOR_EXT2 is not set
> # CONFIG_EXT4_FS_POSIX_ACL is not set
> # CONFIG_EXT4_FS_SECURITY is not set
> # CONFIG_EXT4_DEBUG is not set
>
> using e2fsprogs-1.44.6. Do you mean people now need to update the kernel to
> enable additional config to avoid the spam of warnings now?
Strange. The defaults for mkfs.ext4 _should_ default to use options that
allow enough space for the extra timestamps.
Can you please provide "dumpe2fs -h" output for your filesystem, and the
formatting options that you used when creating this filesystem.
Cheers, Andreas
On Tue, Sep 3, 2019 at 9:39 PM Andreas Dilger <[email protected]> wrote:
>
> On Sep 3, 2019, at 12:15 PM, Qian Cai <[email protected]> wrote:
> >
> > On Tue, 2019-09-03 at 09:36 -0700, Deepa Dinamani wrote:
> >> We might also want to consider updating the file system the LTP is
> >> being run on here.
> >
> > It simply format (mkfs.ext4) a loop back device on ext4 with the kernel.
> >
> > CONFIG_EXT4_FS=m
> > # CONFIG_EXT4_USE_FOR_EXT2 is not set
> > # CONFIG_EXT4_FS_POSIX_ACL is not set
> > # CONFIG_EXT4_FS_SECURITY is not set
> > # CONFIG_EXT4_DEBUG is not set
> >
> > using e2fsprogs-1.44.6. Do you mean people now need to update the kernel to
> > enable additional config to avoid the spam of warnings now?
>
> Strange. The defaults for mkfs.ext4 _should_ default to use options that
> allow enough space for the extra timestamps.
>
> Can you please provide "dumpe2fs -h" output for your filesystem, and the
> formatting options that you used when creating this filesystem.
According to the man page,
"The default inode size is controlled by the mke2fs.conf(5)
file. In the
mke2fs.conf file shipped with e2fsprogs, the default inode size is 256
bytes for most file systems, except for small file systems
where the inode
size will be 128 bytes."
If this (small file systems) is the problem, then I think we need to
do two things:
1. Change the per-inode warning to not warn if the inode size for the
file system is less than 256. We already get a mount-time warning
in that case.
2. Change the mkfs.ext4 defaults to never pick a 128 byte inode unless
the user really wants this (maybe not even then).
Arnd
On Tue, 2019-09-03 at 21:50 +0200, Arnd Bergmann wrote:
> On Tue, Sep 3, 2019 at 9:39 PM Andreas Dilger <[email protected]> wrote:
> >
> > On Sep 3, 2019, at 12:15 PM, Qian Cai <[email protected]> wrote:
> > >
> > > On Tue, 2019-09-03 at 09:36 -0700, Deepa Dinamani wrote:
> > > > We might also want to consider updating the file system the LTP is
> > > > being run on here.
> > >
> > > It simply format (mkfs.ext4) a loop back device on ext4 with the kernel.
> > >
> > > CONFIG_EXT4_FS=m
> > > # CONFIG_EXT4_USE_FOR_EXT2 is not set
> > > # CONFIG_EXT4_FS_POSIX_ACL is not set
> > > # CONFIG_EXT4_FS_SECURITY is not set
> > > # CONFIG_EXT4_DEBUG is not set
> > >
> > > using e2fsprogs-1.44.6. Do you mean people now need to update the kernel
> > > to
> > > enable additional config to avoid the spam of warnings now?
> >
> > Strange. The defaults for mkfs.ext4 _should_ default to use options that
> > allow enough space for the extra timestamps.
> >
> > Can you please provide "dumpe2fs -h" output for your filesystem, and the
> > formatting options that you used when creating this filesystem.
>
> According to the man page,
>
> "The default inode size is controlled by the mke2fs.conf(5)
> file. In the
> mke2fs.conf file shipped with e2fsprogs, the default inode size is
> 256
> bytes for most file systems, except for small file systems
> where the inode
> size will be 128 bytes."
>
> If this (small file systems) is the problem, then I think we need to
> do two things:
>
> 1. Change the per-inode warning to not warn if the inode size for the
> file system is less than 256. We already get a mount-time warning
> in that case.
>
> 2. Change the mkfs.ext4 defaults to never pick a 128 byte inode unless
> the user really wants this (maybe not even then).
Indeed.
# dd if=/dev/zero of=small bs=1M count=50
50+0 records in
50+0 records out
52428800 bytes (52 MB, 50 MiB) copied, 0.0168322 s, 3.1 GB/s
# losetup -f small
# mkfs.ext4 /dev/loop0
# dumpe2fs -h /dev/loop0
dumpe2fs 1.44.6 (5-Mar-2019)
Filesystem volume name: <none>
Last mounted on: <not available>
Filesystem UUID: 8cd1b7f1-dec9-45fc-807b-26cceedcdaa7
Filesystem magic number: 0xEF53
Filesystem revision #: 1 (dynamic)
Filesystem features: has_journal ext_attr resize_inode dir_index filetype
extent 64bit flex_bg sparse_super large_file huge_file dir_nlink extra_isize
metadata_csum
Filesystem flags: unsigned_directory_hash
Default mount options: user_xattr acl
Filesystem state: clean
Errors behavior: Continue
Filesystem OS type: Linux
Inode count: 12824
Block count: 51200
Reserved block count: 2560
Free blocks: 44440
Free inodes: 12813
First block: 1
Block size: 1024
Fragment size: 1024
Group descriptor size: 64
Reserved GDT blocks: 256
Blocks per group: 8192
Fragments per group: 8192
Inodes per group: 1832
Inode blocks per group: 229
Flex block group size: 16
Filesystem created: Tue Sep 3 16:10:35 2019
Last mount time: Tue Sep 3 16:10:42 2019
Last write time: Tue Sep 3 16:10:48 2019
Mount count: 1
Maximum mount count: -1
Last checked: Tue Sep 3 16:10:35 2019
Check interval: 0 (<none>)
Lifetime writes: 6050 kB
Reserved blocks uid: 0 (user root)
Reserved blocks gid: 0 (group root)
First inode: 11
Inode size: 128
Journal inode: 8
Default directory hash: half_md4
Directory Hash Seed: 6507a815-ee3a-4573-99c8-2f9103061dec
Journal backup: inode blocks
Checksum type: crc32c
Checksum: 0x4b0ec46e
Journal features: journal_64bit journal_checksum_v3
Journal size: 4096k
Journal length: 4096
Journal sequence: 0x00000004
Journal start: 0
Journal checksum type: crc32c
Journal checksum: 0x23f8be20
On Tue, Sep 03, 2019 at 09:18:44AM -0700, Deepa Dinamani wrote:
>
> This prints a warning for each inode that doesn't extend limits beyond
> 2038. It is rate limited by the ext4_warning_inode().
> Looks like your filesystem has inodes that cannot be extended.
> We could use a different rate limit or ignore this corner case. Do the
> maintainers have a preference?
We need to drop this commit (ext4: Initialize timestamps limits), or
at least the portion which adds the call to the EXT4_INODE_SET_XTIME
macro in ext4.h.
I know of a truly vast number of servers in production all over the
world which are using 128 byte inodes, and spamming the inodes at the
maximum rate limit is a really bad idea. This includes at some major
cloud data centers where the life of individual servers in their data
centers is well understood (they're not going to last until 2038) and
nothing stored on the local Linux file systems are long-lived ---
that's all stored in the cluster file systems. The choice of 128 byte
inode was deliberately chosen to maximize storage TCO, and so spamming
a warning at high rates is going to be extremely unfriendly.
In cases where the inode size is such that there is no chance at all
to support timestamps beyond 2038, a single warning at mount time, or
maybe a warning at mkfs time might be acceptable. But there's no
point printing a warning time each time we set a timestamp on such a
file system. It's not going to change, and past a certain point, we
need to trust that people who are using 128 byte inodes did so knowing
what the tradeoffs might be. After all, it is *not* the default.
- Ted
On Tue, Sep 3, 2019 at 11:31 PM Deepa Dinamani <[email protected]> wrote:
> On Tue, Sep 3, 2019 at 2:18 PM Theodore Y. Ts'o <[email protected]> wrote:
> > On Tue, Sep 03, 2019 at 09:18:44AM -0700, Deepa Dinamani wrote:
> > >
> > > This prints a warning for each inode that doesn't extend limits beyond
> > > 2038. It is rate limited by the ext4_warning_inode().
> > > Looks like your filesystem has inodes that cannot be extended.
> > > We could use a different rate limit or ignore this corner case. Do the
> > > maintainers have a preference?
> >
> > We need to drop this commit (ext4: Initialize timestamps limits), or
> > at least the portion which adds the call to the EXT4_INODE_SET_XTIME
> > macro in ext4.h.
>
> As Arnd said, I think this can be fixed by warning only when the inode
> size is not uniformly 128 bytes in ext4.h. Is this an acceptable
> solution or we want to drop this warning altogether?
I think the warning as it was intended makes sense, the idea
was never to warn on every inode update for file systems that
cannot handle future dates, only to warn when we
a) try to set a future date
b) fail to do that because the space cannot be made available.
> Arnd, should I be sending a pull request again with the fix? Or, we
> drop the ext4 patch and I can send it to the maintainers directly?
I would prefer to fix it on top of the patches I already merged.
Maybe something like:
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9e3ae3be3de9..5a971d1b6d5e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -835,7 +835,9 @@ do {
\
}
\
else {\
(raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t,
(inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
- ext4_warning_inode(inode, "inode does not support
timestamps beyond 2038"); \
+ if (((inode)->xtime.tv_sec != (raw_inode)->xtime) && \
+ ((inode)->i_sb->s_time_max > S32_MAX))
\
+ ext4_warning_inode(inode, "inode does not
support timestamps beyond 2038"); \
} \
} while (0)
> > In cases where the inode size is such that there is no chance at all
> > to support timestamps beyond 2038, a single warning at mount time, or
> > maybe a warning at mkfs time might be acceptable. But there's no
> > point printing a warning time each time we set a timestamp on such a
> > file system. It's not going to change, and past a certain point, we
> > need to trust that people who are using 128 byte inodes did so knowing
> > what the tradeoffs might be. After all, it is *not* the default.
>
> We have a single mount time warning already in place here. I did not
> realize some people actually chose to use 128 byte inodes on purpose.
This is also new to me, as I always assumed a normal ext4 would be y2038
safe. I suspect that a few of those users are unaware of the y2038
problem they might run into because of that, but that's what the mount-time
warning should help with.
However, I did expect that people might have legacy ext3 file system
images that they mount, and printing a warning for each write would
also be wrong for those.
Arnd
Am 03.09.19 um 23:17 schrieb Theodore Y. Ts'o:
> I know of a truly vast number of servers in production all over the
> world which are using 128 byte inodes, and spamming the inodes at the
> maximum rate limit is a really bad idea. This includes at some major
> cloud data centers where the life of individual servers in their data
> centers is well understood (they're not going to last until 2038)
well, i didn't ask the Fedora installer for 128 byte indoes in 2008 on
the 500 MB small /boot while the 6 GB rootfs has 256 byte while this
setups are surely targeted to last longer than 2038 until someone kills
Fedora with all the shiny new stuff nobody needs
but yes, don't start to spam me about it
[root@arrakis:~]$ tune2fs -l /dev/sda1
tune2fs 1.44.6 (5-Mar-2019)
Filesystem volume name: boot
Last mounted on: /boot
Filesystem UUID: b834776d-69d1-49c6-97c1-d6d758a438f0
Filesystem magic number: 0xEF53
Filesystem revision #: 1 (dynamic)
Filesystem features: has_journal ext_attr resize_inode dir_index
filetype needs_recovery extent sparse_super uninit_bg
Filesystem flags: signed_directory_hash
Default mount options: (none)
Filesystem state: clean
Errors behavior: Continue
Filesystem OS type: Linux
Inode count: 130560
Block count: 521215
Reserved block count: 2
Free blocks: 455376
Free inodes: 130216
First block: 1
Block size: 1024
Fragment size: 1024
Reserved GDT blocks: 256
Blocks per group: 8192
Fragments per group: 8192
Inodes per group: 2040
Inode blocks per group: 255
Filesystem created: Mon Aug 18 06:48:14 2008
Last mount time: Sat Aug 17 02:49:03 2019
Last write time: Tue Sep 3 02:03:44 2019
Mount count: 19
Maximum mount count: 30
Last checked: Sat Dec 15 04:36:27 2018
Check interval: 31104000 (12 months)
Next check after: Tue Dec 10 04:36:27 2019
Lifetime writes: 64 GB
Reserved blocks uid: 0 (user root)
Reserved blocks gid: 0 (group root)
First inode: 11
Inode size: 128
Journal inode: 8
Default directory hash: half_md4
Directory Hash Seed: 2cc862b9-dc3e-4707-b6ed-9a7fe724dd2e
Journal backup: inode blocks
On Tue, Sep 03, 2019 at 11:48:14PM +0200, Arnd Bergmann wrote:
> I think the warning as it was intended makes sense, the idea
> was never to warn on every inode update for file systems that
> cannot handle future dates, only to warn when we
>
> a) try to set a future date
> b) fail to do that because the space cannot be made available.
What do you mean by "try to set a future date"? Do you mean a trying
to set a date after 2038 (when it can no longer fit in a signed 32-bit
value)? Because that's not what the commit is currently doing.....
> I would prefer to fix it on top of the patches I already merged.
>
> Maybe something like:
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9e3ae3be3de9..5a971d1b6d5e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -835,7 +835,9 @@ do {
> \
> }
> \
> else {\
> (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t,
> (inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
> - ext4_warning_inode(inode, "inode does not support
> timestamps beyond 2038"); \
> + if (((inode)->xtime.tv_sec != (raw_inode)->xtime) && \
> + ((inode)->i_sb->s_time_max > S32_MAX))
> \
> + ext4_warning_inode(inode, "inode does not
> support timestamps beyond 2038"); \
> } \
> } while (0)
Sure, that's much less objectionable.
> However, I did expect that people might have legacy ext3 file system
> images that they mount, and printing a warning for each write would
> also be wrong for those.
I guess I'm much less convinced that 10-15 years from now, there will
be many legacy ext3 file systems left. Storage media doesn't last
that long, and if file systems get moved around, e2fsck will be run at
least once, and so adding some e2fsck-time warnings seems to be a
better approach IMHO.
- Ted
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 9e3ae3be3de9..5a971d1b6d5e 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -835,7 +835,9 @@ do {
> > \
> > }
> > \
> > else {\
> > (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t,
> > (inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
> > - ext4_warning_inode(inode, "inode does not support
> > timestamps beyond 2038"); \
> > + if (((inode)->xtime.tv_sec != (raw_inode)->xtime) && \
> > + ((inode)->i_sb->s_time_max > S32_MAX))
> > \
> > + ext4_warning_inode(inode, "inode does not
> > support timestamps beyond 2038"); \
> > } \
> > } while (0)
>
> Sure, that's much less objectionable.
The reason it was warning for every update was because of the
ratelimiting. I think ratelimiting is not working well here. I will
check that part.
-Deepa
If we don't care to warn about the timestamps that are clamped in
memory, maybe we could just warn when they are being written out.
Would something like this be more acceptable? I would also remove the
warning in ext4.h. I think we don't have to check if the inode is 128
bytes here (Please correct me if I am wrong). If this looks ok, I can
post this.
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9e3ae3be3de9..24b14bd3feab 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -833,10 +833,8 @@ do {
\
(raw_inode)->xtime ## _extra =
\
ext4_encode_extra_time(&(inode)->xtime); \
}
\
- else {\
+ else \
(raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t,
(inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
- ext4_warning_inode(inode, "inode does not support
timestamps beyond 2038"); \
- } \
} while (0)
#define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode)
\
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 491f9ee4040e..cef5b87cc5a6 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -2791,7 +2791,7 @@ int ext4_expand_extra_isize_ea(struct inode
*inode, int new_extra_isize,
cleanup:
if (error && (mnt_count != le16_to_cpu(sbi->s_es->s_mnt_count))) {
- ext4_warning(inode->i_sb, "Unable to expand inode %lu.
Delete some EAs or run e2fsck.",
+ ext4_warning(inode->i_sb, "Unable to expand inode %lu.
Delete some EAs or run e2fsck. Timestamps on the inode expire beyond
2038",
inode->i_ino);
mnt_count = le16_to_cpu(sbi->s_es->s_mnt_count);
}
On Tue, Sep 03, 2019 at 09:50:09PM -0700, Deepa Dinamani wrote:
> If we don't care to warn about the timestamps that are clamped in
> memory, maybe we could just warn when they are being written out.
> Would something like this be more acceptable? I would also remove the
> warning in ext4.h. I think we don't have to check if the inode is 128
> bytes here (Please correct me if I am wrong). If this looks ok, I can
> post this.
That's better, but it's going to be misleading in many cases. The
inode's extra size field is 16 or larger, there will be enough space
for the timestamps, so talking about "timestamps on this inode beyond
2038" when ext4 is unable to expand it from say, 24 to 32, won't be
true. Certain certain features won't be available, yes --- such as
project-id-based quotas, since there won't be room to store the
project ID. However, it's not going to impact the ability to store
timestamps beyond 2038. The i_extra_isize field is not just about
timestamps!
Again, the likelihood that there will be file systems that have this
problem in 2038 is... extremely low in my judgement. Storage media
just doesn't last that long; and distributions such as Red Hat and
SuSE very strongly encourage people to reformat file systems and do
*not* support upgrades from ext3 to ext4 by using tune2fs. If you do
this, their help desk will laugh at you and refuse to help you.
Companies like Google will do this kind of upgrades[1], sure. But
that's because backing up and reformatting vast numbers of file
systems are not practical at scale. (And even Google doesn't maintain
the file system image when the servers are old enough to be TCO
negative and it's time to replace them.)
In contrast, most companies / users don't do this sort of thing at
all. It's not an issue for Cell Phones, for example, or most consumer
devices, which are lucky if the last more than 3 years before they get
desupported and stop getting security updates, and then the lithium
ion batttery dies and the device end up in a landfill. Those that
might live 20 years (although good luck with that for something like,
say, a smart thermostat) aren't going to have a console and no one
will be paying attention to the kernel messages anyway. So is it
really worth it? For whom are these messages meant?
[1] https://www.youtube.com/watch?v=Wp5Ehw7ByuU
Cheers,
- Ted
Am 04.09.19 um 14:58 schrieb Theodore Y. Ts'o:
> Again, the likelihood that there will be file systems that have this
> problem in 2038 is... extremely low in my judgement. Storage media
> just doesn't last that long
in times of virtualization storage media are below the vdisk and the
file system lasts that long and even longer
in times of running RAID on your storage they last that long because you
happily replace dead disks and move you hard drives to the next computer
when the rest of the hardware is dead
> and distributions such as Red Hat and
> SuSE very strongly encourage people to reformat file systems and do
> *not* support upgrades from ext3 to ext4 by using tune2fs. If you do
> this, their help desk will laugh at you and refuse to help you.
i would have laughed at somebody telling me in 2010 that i have to start
again from scratch instead convert all the virtual servers installed two
years ago to ext4 or in general install repeatly from scratch instead
doing all the dist-upgrades from Fedora 9 to Fedora 30 with no downtime
longer than a ordinary kernel update and reboot
and here we are, with file systems and operating systems installed in
2008 running 11 years later just fine - it's Linux not Windows
> On Sep 4, 2019, at 5:58 AM, Theodore Y. Ts'o <[email protected]> wrote:
>
>> On Tue, Sep 03, 2019 at 09:50:09PM -0700, Deepa Dinamani wrote:
>> If we don't care to warn about the timestamps that are clamped in
>> memory, maybe we could just warn when they are being written out.
>> Would something like this be more acceptable? I would also remove the
>> warning in ext4.h. I think we don't have to check if the inode is 128
>> bytes here (Please correct me if I am wrong). If this looks ok, I can
>> post this.
>
> That's better, but it's going to be misleading in many cases. The
> inode's extra size field is 16 or larger, there will be enough space
> for the timestamps, so talking about "timestamps on this inode beyond
> 2038" when ext4 is unable to expand it from say, 24 to 32, won't be
> true. Certain certain features won't be available, yes --- such as
> project-id-based quotas, since there won't be room to store the
> project ID. However, it's not going to impact the ability to store
> timestamps beyond 2038. The i_extra_isize field is not just about
> timestamps!
I understand that i_extra_isize is not just about timestamps. It’s
evident from EXT4_FITS_IN_INODE(). I think we can check for
EXT4_FITS_IN_INODE() here if that will consistently eliminates false
positives.
But, I hear you. You think this warning is unnecessary. I think there
are many file systems and I don’t think anybody would knows in’s and
outs of each one. I think if I’m mounting an ext4 fs and it has mixed
sizes of inodes, I think I would at least expect a dmesg(with a hint
on how to fix it) considering that this filesystem is restricted in
more ways than just time. Is this the purpose of the warning you
already have?:
if (error && (mnt_count != le16_to_cpu(sbi->s_es->s_mnt_count))) {
ext4_warning(inode->i_sb, "Unable to expand inode %lu.
Delete some EAs or run e2fsck.",
Maybe there should be a warning, but it has nothing to do with just
time. Do we already have this?
-Deepa
When ext4 file systems were created intentionally with 128 byte inodes,
the rate-limited warning of eventual possible timestamp overflow are
still emitted rather frequently. Remove the warning for now.
Discussion for whether any warning is needed,
and where it should be emitted, can be found at
https://lore.kernel.org/lkml/[email protected]/.
I can post a separate follow-up patch after the conclusion.
Reported-by: Qian Cai <[email protected]>
Signed-off-by: Deepa Dinamani <[email protected]>
---
fs/ext4/ext4.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 9e3ae3be3de9..24b14bd3feab 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -833,10 +833,8 @@ do { \
(raw_inode)->xtime ## _extra = \
ext4_encode_extra_time(&(inode)->xtime); \
} \
- else {\
+ else \
(raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
- ext4_warning_inode(inode, "inode does not support timestamps beyond 2038"); \
- } \
} while (0)
#define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode) \
--
2.17.1
On Sep 4, 2019, at 09:02, Deepa Dinamani <[email protected]> wrote:
>
> When ext4 file systems were created intentionally with 128 byte inodes,
> the rate-limited warning of eventual possible timestamp overflow are
> still emitted rather frequently. Remove the warning for now.
>
> Discussion for whether any warning is needed,
> and where it should be emitted, can be found at
> https://lore.kernel.org/lkml/[email protected]/.
> I can post a separate follow-up patch after the conclusion.
>
> Reported-by: Qian Cai <[email protected]>
> Signed-off-by: Deepa Dinamani <[email protected]>
I'd be in favor of a severely rare-limited warning in the actual case
that Y2038 timestamps cannot be stored, but the current message is
too verbose for now and I agree it is better to remove it while discussions
on the best solution are underway.
Reviewed-by: Andreas Dilger <[email protected]>
> ---
> fs/ext4/ext4.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 9e3ae3be3de9..24b14bd3feab 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -833,10 +833,8 @@ do { \
> (raw_inode)->xtime ## _extra = \
> ext4_encode_extra_time(&(inode)->xtime); \
> } \
> - else {\
> + else \
> (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (inode)->xtime.tv_sec, S32_MIN, S32_MAX)); \
> - ext4_warning_inode(inode, "inode does not support timestamps beyond 2038"); \
> - } \
> } while (0)
>
> #define EXT4_EINODE_SET_XTIME(xtime, einode, raw_inode) \
> --
> 2.17.1
>
On Wed, Sep 4, 2019 at 8:39 PM Andreas Dilger <[email protected]> wrote:
>
> On Sep 4, 2019, at 09:02, Deepa Dinamani <[email protected]> wrote:
> >
> > When ext4 file systems were created intentionally with 128 byte inodes,
> > the rate-limited warning of eventual possible timestamp overflow are
> > still emitted rather frequently. Remove the warning for now.
> >
> > Discussion for whether any warning is needed,
> > and where it should be emitted, can be found at
> > https://lore.kernel.org/lkml/[email protected]/.
> > I can post a separate follow-up patch after the conclusion.
> >
> > Reported-by: Qian Cai <[email protected]>
> > Signed-off-by: Deepa Dinamani <[email protected]>
>
> I'd be in favor of a severely rare-limited warning in the actual case
> that Y2038 timestamps cannot be stored, but the current message is
> too verbose for now and I agree it is better to remove it while discussions
> on the best solution are underway.
>
> Reviewed-by: Andreas Dilger <[email protected]>
Agreed completely.
Applied on top of the y2038 branch now, thanks a lot for the update!
This should be part of tomorrow's linux-next then.
Arnd