ext4 has different overflow limits for max filesystem
timestamps based on the extra bytes available.
The timestamp limits are calculated according to the
encoding table in
a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
* extra msb of adjust for signed
* epoch 32-bit 32-bit tv_sec to
* bits time decoded 64-bit tv_sec 64-bit tv_sec valid time range
* 0 0 1 -0x80000000..-0x00000001 0x000000000 1901-12-13..1969-12-31
* 0 0 0 0x000000000..0x07fffffff 0x000000000 1970-01-01..2038-01-19
* 0 1 1 0x080000000..0x0ffffffff 0x100000000 2038-01-19..2106-02-07
* 0 1 0 0x100000000..0x17fffffff 0x100000000 2106-02-07..2174-02-25
* 1 0 1 0x180000000..0x1ffffffff 0x200000000 2174-02-25..2242-03-16
* 1 0 0 0x200000000..0x27fffffff 0x200000000 2242-03-16..2310-04-04
* 1 1 1 0x280000000..0x2ffffffff 0x300000000 2310-04-04..2378-04-22
* 1 1 0 0x300000000..0x37fffffff 0x300000000 2378-04-22..2446-05-10
Note that the time limits are not correct for deletion times.
Signed-off-by: Deepa Dinamani <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
fs/ext4/ext4.h | 4 ++++
fs/ext4/super.c | 17 +++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1cb67859e051..3f13cf12ae7f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1631,6 +1631,10 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
#define EXT4_GOOD_OLD_INODE_SIZE 128
+#define EXT4_EXTRA_TIMESTAMP_MAX (((s64)1 << 34) - 1 + S32_MIN)
+#define EXT4_NON_EXTRA_TIMESTAMP_MAX S32_MAX
+#define EXT4_TIMESTAMP_MIN S32_MIN
+
/*
* Feature set definitions
*/
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4079605d437a..3ea2d60f33aa 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4035,8 +4035,21 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_inode_size);
goto failed_mount;
}
- if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE)
- sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
+ /*
+ * i_atime_extra is the last extra field available for [acm]times in
+ * struct ext4_inode. Checking for that field should suffice to ensure
+ * we have extra space for all three.
+ */
+ if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
+ sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
+ sb->s_time_gran = 1;
+ sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
+ } else {
+ sb->s_time_gran = NSEC_PER_SEC;
+ sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
+ }
+
+ sb->s_time_min = EXT4_TIMESTAMP_MIN;
}
sbi->s_desc_size = le16_to_cpu(es->s_desc_size);
--
2.17.1
On Mon, Jul 29, 2019 at 06:49:13PM -0700, Deepa Dinamani wrote:
> ext4 has different overflow limits for max filesystem
> timestamps based on the extra bytes available.
>
> The timestamp limits are calculated according to the
> encoding table in
> a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
>
> * extra msb of adjust for signed
> * epoch 32-bit 32-bit tv_sec to
> * bits time decoded 64-bit tv_sec 64-bit tv_sec valid time range
> * 0 0 1 -0x80000000..-0x00000001 0x000000000 1901-12-13..1969-12-31
> * 0 0 0 0x000000000..0x07fffffff 0x000000000 1970-01-01..2038-01-19
> * 0 1 1 0x080000000..0x0ffffffff 0x100000000 2038-01-19..2106-02-07
> * 0 1 0 0x100000000..0x17fffffff 0x100000000 2106-02-07..2174-02-25
> * 1 0 1 0x180000000..0x1ffffffff 0x200000000 2174-02-25..2242-03-16
> * 1 0 0 0x200000000..0x27fffffff 0x200000000 2242-03-16..2310-04-04
> * 1 1 1 0x280000000..0x2ffffffff 0x300000000 2310-04-04..2378-04-22
> * 1 1 0 0x300000000..0x37fffffff 0x300000000 2378-04-22..2446-05-10
My recollection of ext4 has gotten rusty, so this could be a bogus
question:
Say you have a filesystem with s_inode_size > 128 where not all of the
ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
don't support nanoseconds or times beyond 2038. I think this happens on
ext3 filesystems that reserved extra space for inode attrs that are
subsequently converted to ext4?
In any case, that means that you have some inodes that support 34-bit
tv_sec and some inodes that only support 32-bit tv_sec. For the inodes
with 32-bit tv_sec, I think all that happens is that a large timestamp
will be truncated further, right?
And no mount time warning because at least /some/ of the inodes are
ready to go for the next 30 years?
--D
> Note that the time limits are not correct for deletion times.
>
> Signed-off-by: Deepa Dinamani <[email protected]>
> Reviewed-by: Andreas Dilger <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> fs/ext4/ext4.h | 4 ++++
> fs/ext4/super.c | 17 +++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1cb67859e051..3f13cf12ae7f 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1631,6 +1631,10 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
>
> #define EXT4_GOOD_OLD_INODE_SIZE 128
>
> +#define EXT4_EXTRA_TIMESTAMP_MAX (((s64)1 << 34) - 1 + S32_MIN)
> +#define EXT4_NON_EXTRA_TIMESTAMP_MAX S32_MAX
> +#define EXT4_TIMESTAMP_MIN S32_MIN
> +
> /*
> * Feature set definitions
> */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 4079605d437a..3ea2d60f33aa 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4035,8 +4035,21 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> sbi->s_inode_size);
> goto failed_mount;
> }
> - if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE)
> - sb->s_time_gran = 1 << (EXT4_EPOCH_BITS - 2);
> + /*
> + * i_atime_extra is the last extra field available for [acm]times in
> + * struct ext4_inode. Checking for that field should suffice to ensure
> + * we have extra space for all three.
> + */
> + if (sbi->s_inode_size >= offsetof(struct ext4_inode, i_atime_extra) +
> + sizeof(((struct ext4_inode *)0)->i_atime_extra)) {
> + sb->s_time_gran = 1;
> + sb->s_time_max = EXT4_EXTRA_TIMESTAMP_MAX;
> + } else {
> + sb->s_time_gran = NSEC_PER_SEC;
> + sb->s_time_max = EXT4_NON_EXTRA_TIMESTAMP_MAX;
> + }
> +
> + sb->s_time_min = EXT4_TIMESTAMP_MIN;
> }
>
> sbi->s_desc_size = le16_to_cpu(es->s_desc_size);
> --
> 2.17.1
>
On Wed, Jul 31, 2019 at 8:26 AM Darrick J. Wong <[email protected]> wrote:
>
> On Mon, Jul 29, 2019 at 06:49:13PM -0700, Deepa Dinamani wrote:
> > ext4 has different overflow limits for max filesystem
> > timestamps based on the extra bytes available.
> >
> > The timestamp limits are calculated according to the
> > encoding table in
> > a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
> >
> > * extra msb of adjust for signed
> > * epoch 32-bit 32-bit tv_sec to
> > * bits time decoded 64-bit tv_sec 64-bit tv_sec valid time range
> > * 0 0 1 -0x80000000..-0x00000001 0x000000000 1901-12-13..1969-12-31
> > * 0 0 0 0x000000000..0x07fffffff 0x000000000 1970-01-01..2038-01-19
> > * 0 1 1 0x080000000..0x0ffffffff 0x100000000 2038-01-19..2106-02-07
> > * 0 1 0 0x100000000..0x17fffffff 0x100000000 2106-02-07..2174-02-25
> > * 1 0 1 0x180000000..0x1ffffffff 0x200000000 2174-02-25..2242-03-16
> > * 1 0 0 0x200000000..0x27fffffff 0x200000000 2242-03-16..2310-04-04
> > * 1 1 1 0x280000000..0x2ffffffff 0x300000000 2310-04-04..2378-04-22
> > * 1 1 0 0x300000000..0x37fffffff 0x300000000 2378-04-22..2446-05-10
>
> My recollection of ext4 has gotten rusty, so this could be a bogus
> question:
>
> Say you have a filesystem with s_inode_size > 128 where not all of the
> ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> don't support nanoseconds or times beyond 2038. I think this happens on
> ext3 filesystems that reserved extra space for inode attrs that are
> subsequently converted to ext4?
>
> In any case, that means that you have some inodes that support 34-bit
> tv_sec and some inodes that only support 32-bit tv_sec. For the inodes
> with 32-bit tv_sec, I think all that happens is that a large timestamp
> will be truncated further, right?
>
> And no mount time warning because at least /some/ of the inodes are
> ready to go for the next 30 years?
I'm confused about ext3 being converted to ext4. If the converted
inodes have extra space, then ext4_iget() will start using the extra
space when it modifies the on disk inode, won't it?
But, if there are 32 bit tv_sec and 34 bit tv_sec in a superblock then
from this macro below, if an inode has space for extra bits in
timestamps, it uses it. Otherwise, only the first 32 bits are copied
to the on disk timestamp. This matches the behavior today for 32 bit
tv_sec. But, the 34 bit tv_sec has a better behavior after the series
because of the clamping and warning.
#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 ## _extra = \
ext4_encode_extra_time(&(inode)->xtime); \
} \
} while (0)
I'm not sure if this corner case if important. Maybe the maintainers
can help me here. If this is important, then the inode time updates
for an ext4 inode should always be through ext4_setattr() and we can
clamp the timestamps there as a special case. And, this patch can be
added separately?
Thanks,
Deepa
On Thu, Aug 01, 2019 at 12:18:28PM -0700, Deepa Dinamani wrote:
> > Say you have a filesystem with s_inode_size > 128 where not all of the
> > ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> > don't support nanoseconds or times beyond 2038. I think this happens on
> > ext3 filesystems that reserved extra space for inode attrs that are
> > subsequently converted to ext4?
>
> I'm confused about ext3 being converted to ext4. If the converted
> inodes have extra space, then ext4_iget() will start using the extra
> space when it modifies the on disk inode, won't it?i
It is possible that you can have an ext3 file system with (for
example) 256 byte inodes, and all of the extra space was used for
extended attributes, then ext4 won't have the extra space available.
This is going toh be on an inode-by-inode basis, and if an extended
attribute is motdified or deleted, the space would become available,t
and then inode would start getting a higher resolution timestamp.
I really don't think it's worth worrying about that, though. It's
highly unlikely ext3 file systems will be still be in service by the
time it's needed in 2038. And if so, it's highly unlikely they would
be converted to ext4.
- Ted
On Fri, Aug 2, 2019 at 12:43 AM Theodore Y. Ts'o <[email protected]> wrote:
>
> On Thu, Aug 01, 2019 at 12:18:28PM -0700, Deepa Dinamani wrote:
> > > Say you have a filesystem with s_inode_size > 128 where not all of the
> > > ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> > > don't support nanoseconds or times beyond 2038. I think this happens on
> > > ext3 filesystems that reserved extra space for inode attrs that are
> > > subsequently converted to ext4?
> >
> > I'm confused about ext3 being converted to ext4. If the converted
> > inodes have extra space, then ext4_iget() will start using the extra
> > space when it modifies the on disk inode, won't it?i
>
> It is possible that you can have an ext3 file system with (for
> example) 256 byte inodes, and all of the extra space was used for
> extended attributes, then ext4 won't have the extra space available.
> This is going toh be on an inode-by-inode basis, and if an extended
> attribute is motdified or deleted, the space would become available,t
> and then inode would start getting a higher resolution timestamp.
Is it correct to assume that this kind of file would have to be
created using the ext3.ko file system implementation that was
removed in linux-4.3, but not using ext2.ko or ext4.ko (which
would always set the extended timestamps even in "-t ext2" or
"-t ext3" mode)?
I tried to reproduce this on a modern kernel and with and
moderately old debugfs (1.42.13) but failed.
> I really don't think it's worth worrying about that, though. It's
> highly unlikely ext3 file systems will be still be in service by the
> time it's needed in 2038. And if so, it's highly unlikely they would
> be converted to ext4.
As the difference is easily visible even before y2038 by using
utimensat(old_inode, future_date) on a file, we should at least
decide what the sanest behavior is that we can easily implement,
and then document what is expected to happen here.
If we check for s_min_extra_isize instead of s_inode_size
to determine s_time_gran/s_time_max, we would warn
at mount time as well as and consistently truncate all
timestamps to full 32-bit seconds, regardless of whether
there is actually space or not.
Alternatively, we could warn if s_min_extra_isize is
too small, but use i_inode_size to determine
s_time_gran/s_time_max anyway.
From looking at e2fsprogs git history, I see that
s_min_extra_isize has always been set by mkfs since
2008, but I'm not sure if there would have been a
case in which it remains set but the ext3.ko would
ignore it and use that space anyway.
Arnd
On Fri, Aug 02, 2019 at 12:39:41PM +0200, Arnd Bergmann wrote:
> Is it correct to assume that this kind of file would have to be
> created using the ext3.ko file system implementation that was
> removed in linux-4.3, but not usiing ext2.ko or ext4.ko (which
> would always set the extended timestamps even in "-t ext2" or
> "-t ext3" mode)?
Correct. Some of the enterprise distro's were using ext4 to support
"mount -t ext3" even before 4.3. There's a CONFIG option to enable
using ext4 for ext2 or ext3 if they aren't enabled.
> If we check for s_min_extra_isize instead of s_inode_size
> to determine s_time_gran/s_time_max, we would warn
> at mount time as well as and consistently truncate all
> timestamps to full 32-bit seconds, regardless of whether
> there is actually space or not.
>
> Alternatively, we could warn if s_min_extra_isize is
> too small, but use i_inode_size to determine
> s_time_gran/s_time_max anyway.
Even with ext4, s_min_extra_isize doesn't guarantee that will be able
to expand the inode. This can fail if (a) we aren't able to expand
existing the transaction handle because there isn't enough space in
the journal, or (b) there is already an external xattr block which is
also full, so there is no space to evacuate an extended attribute out
of the inode's extra space.
We could be more aggressive by trying to expand make room in the inode
in ext4_iget (when we're reading in the inode, assuming the file
system isn't mounted read/only), instead of in the middle of
mark_inode_dirty(). That will eliminate failure mode (a) --- which is
statistically rare --- but it won't eliminate failure mode (b).
Ultimately, the question is which is worse: having a timestamp be
wrong, or randomly dropping an xattr from the inode to make room for
the extended timestamp. We've come down on it being less harmful to
have the timestamp be wrong.
But again, this is a pretty rare case. I'm not convinced it's worth
stressing about, since it's going to require multiple things to go
wrong before a timestamp will be bad.
- Ted
On Fri, Aug 02, 2019 at 09:00:52PM +0200, Arnd Bergmann wrote:
>
> I must have misunderstood what the field says. I expected that
> with s_min_extra_isize set beyond the nanosecond fields, there
> would be a guarantee that all inodes have at least as many
> extra bytes already allocated. What circumstances would lead to
> an i_extra_isize smaller than s_min_extra_isize?
When allocating new inodes, i_extra_isize is set to
s_want_extra_isize. When modifying existing inodes, if i_extra_isize
is less than s_min_extra_isize, then we will attempt to move out
extended attribute(s) to the external xattr block. So the
s_min_extra_isize field is not a guarantee, but rather an aspirationa
goal. The idea is that at some point when we want to enable a new
feature, which needs more extra inode space, we can adjust
s_min_extra_size and s_want_extra_size, and the file system will
migrate things to meet these constraints.
The plan was to teach e2fsck how to fix all of the inodes to meet theh
s_min_extra_size value, but that never got implemented, and we even
then, e2fsck would have to deal with the case where tit couldn't move
the extended attribute(s) in the inode out, because there was no place
to put them.
In practice, this hasn't been that much of a limitation because we
haven't been adding that many extra inode fields. Keep in mind that
Red Hat for example, has explicitly said they will *never* support
adding new features to an existing file system. Their only supported
method is back up the file system, reformat it with the new file
system features, and then restore the file system.
Of course, if the backup/restore includes backing up the extended
attributes, and then restoring them, the xattr restore could fail,
unless the user also increased the inode size (e.g., from 256 bytes to
512 bytes).
Getting this right in the general case is *hard*. Fortunately, the
corner cases really don't happen that often in practice, at least not
for pure Linux workloads. Windows which can have arbitrarily large
security id's and ACL's might make this harder, of course --- although
ext4's EA in inode feature would make this better, modulo needing to
write more complex file system code to handle moving xattrs around.
Since the extended timestamps were one of the first extra inode fields
to be added, I strongly suggest that we not try to borrow trouble.
Solving the general case problem is *hard*.
- Ted
On Fri, Aug 2, 2019 at 11:39 PM Theodore Y. Ts'o <[email protected]> wrote:
>
> On Fri, Aug 02, 2019 at 09:00:52PM +0200, Arnd Bergmann wrote:
> >
> > I must have misunderstood what the field says. I expected that
> > with s_min_extra_isize set beyond the nanosecond fields, there
> > would be a guarantee that all inodes have at least as many
> > extra bytes already allocated. What circumstances would lead to
> > an i_extra_isize smaller than s_min_extra_isize?
>
> When allocating new inodes, i_extra_isize is set to
> s_want_extra_isize. When modifying existing inodes, if i_extra_isize
> is less than s_min_extra_isize, then we will attempt to move out
> extended attribute(s) to the external xattr block. So the
> s_min_extra_isize field is not a guarantee, but rather an aspirationa
> goal. The idea is that at some point when we want to enable a new
> feature, which needs more extra inode space, we can adjust
> s_min_extra_size and s_want_extra_size, and the file system will
> migrate things to meet these constraints.
I see in the ext4 code that we always try to expand i_extra_size
to s_want_extra_isize in ext4_mark_inode_dirty(), and that
s_want_extra_isize is always at least s_min_extra_isize, so
we constantly try to expand the inode to fit.
What I still don't see is how any inode on the file system
image could have ended up with less than s_min_extra_isize
in the first place if s_min_extra_isize is never modified and
all inodes in the file system would have originally been
created with i_extra_isize >= s_min_extra_isize if
EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE is set.
Did older versions of ext4 or ext3 ignore s_min_extra_isize
when creating inodes despite
EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
or is there another possibility I'm missing?
> Since the extended timestamps were one of the first extra inode fields
> to be added, I strongly suggest that we not try to borrow trouble.
> Solving the general case problem is *hard*.
As I said before, I absolutely don't suggest we solve the problem
of reliably setting the timestamps, I'm just trying to find out if there
is a way to know for sure that it cannot happen and alert the user
otherwise. So far I think we have concluded
- just checking s_inode_size is not sufficient because ext3
may have created inodes with s_extra_isize too small
- checking s_min_extra_isize may not be sufficient either, for
similar reasons I don't yet fully understand (see above).
If there is any other way to be sure that the file system
has never been mounted as a writable ext3, maybe that can
be used instead?
Arnd
On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
>
> I see in the ext4 code that we always try to expand i_extra_size
> to s_want_extra_isize in ext4_mark_inode_dirty(), and that
> s_want_extra_isize is always at least s_min_extra_isize, so
> we constantly try to expand the inode to fit.
Yes, we *try*. But we may not succeed. There may actually be a
problem here if the cause is due to there simply is no space in the
external xattr block, so we might try and try every time we try to
modify that inode, and it would be a performance mess. If it's due to
there being no room in the current transaction, then it's highly
likely it will succeed the next time.
> Did older versions of ext4 or ext3 ignore s_min_extra_isize
> when creating inodes despite
> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
> or is there another possibility I'm missing?
s_min_extra_isize could get changed in order to make room for some new
file system feature --- such as extended timestamps. That's how we
might take an old ext3 file system with an inode size > 128, and try
to evacuate space for extended timestamps, on a best efforts basis.
And since it's best efforts is why Red Hat refuses to support that
case. It'll work 99.9% of the time, but they don't want to deal with
the 0.01% cases showing up at their help desk.
If you want to pretend that file systems never get upgraded, then life
is much simpler. The general approach is that for less-sophisticated
customers (e.g., most people running enterprise distros) file system
upgrades are not a thing. But for sophisticated users, we do try to
make thing work for people who are aware of the risks / caveats /
rough edges. Google won't have been able to upgrade thousands and
thousands of servers in data centers all over the world if we limited
ourselves to Red Hat's support restrictions. Backup / reformat /
restore really isn't a practical rollout strategy for many exabytes of
file systems.
It sounds like your safety checks / warnings are mostly targeted at
low-information customers, no?
- Ted
On Sat, Aug 3, 2019 at 6:03 PM Theodore Y. Ts'o <[email protected]> wrote:
>
> On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
> >
> > I see in the ext4 code that we always try to expand i_extra_size
> > to s_want_extra_isize in ext4_mark_inode_dirty(), and that
> > s_want_extra_isize is always at least s_min_extra_isize, so
> > we constantly try to expand the inode to fit.
>
> Yes, we *try*. But we may not succeed. There may actually be a
> problem here if the cause is due to there simply is no space in the
> external xattr block, so we might try and try every time we try to
> modify that inode, and it would be a performance mess. If it's due to
> there being no room in the current transaction, then it's highly
> likely it will succeed the next time.
>
> > Did older versions of ext4 or ext3 ignore s_min_extra_isize
> > when creating inodes despite
> > EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
> > or is there another possibility I'm missing?
>
> s_min_extra_isize could get changed in order to make room for some new
> file system feature --- such as extended timestamps.
Ok, that explains it. I assumed s_min_extra_isize was meant to
not be modifiable, and did not find a way to change it using the
kernel or tune2fs, but now I can see that debugfs can set it.
> If you want to pretend that file systems never get upgraded, then life
> is much simpler. The general approach is that for less-sophisticated
> customers (e.g., most people running enterprise distros) file system
> upgrades are not a thing. But for sophisticated users, we do try to
> make thing work for people who are aware of the risks / caveats /
> rough edges. Google won't have been able to upgrade thousands and
> thousands of servers in data centers all over the world if we limited
> ourselves to Red Hat's support restrictions. Backup / reformat /
> restore really isn't a practical rollout strategy for many exabytes of
> file systems.
>
> It sounds like your safety checks / warnings are mostly targeted at
> low-information customers, no?
Yes, that seems like a reasonable compromise: just warn based
on s_min_extra_isize, and assume that anyone who used debugfs
to set s_min_extra_isize to a higher value from an ext3 file system
during the migration to ext4 was aware of the risks already.
That leaves the question of what we should set the s_time_gran
and s_time_max to on a superblock with s_min_extra_isize<16
and s_want_extra_isize>=16.
If we base it on s_min_extra_isize, we never try to set a timestamp
later than 2038 and so will never fail, but anyone with a grandfathered
s_min_extra_isize from ext3 won't be able to set extended
timestamps on any files any more. Based on s_want_extra_isize
we would keep the current behavior, but could add a custom
warning in the ext4 code about the small s_min_extra_isize
indicating a theoretical problem.
Arnd
On Aug 3, 2019, at 2:24 PM, Arnd Bergmann <[email protected]> wrote:
>
> On Sat, Aug 3, 2019 at 6:03 PM Theodore Y. Ts'o <[email protected]> wrote:
>>
>> On Sat, Aug 03, 2019 at 11:30:22AM +0200, Arnd Bergmann wrote:
>>>
>>> I see in the ext4 code that we always try to expand i_extra_size
>>> to s_want_extra_isize in ext4_mark_inode_dirty(), and that
>>> s_want_extra_isize is always at least s_min_extra_isize, so
>>> we constantly try to expand the inode to fit.
>>
>> Yes, we *try*. But we may not succeed. There may actually be a
>> problem here if the cause is due to there simply is no space in the
>> external xattr block, so we might try and try every time we try to
>> modify that inode, and it would be a performance mess. If it's due to
>> there being no room in the current transaction, then it's highly
>> likely it will succeed the next time.
>>
>>> Did older versions of ext4 or ext3 ignore s_min_extra_isize
>>> when creating inodes despite
>>> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE,
>>> or is there another possibility I'm missing?
>>
>> s_min_extra_isize could get changed in order to make room for some new
>> file system feature --- such as extended timestamps.
>
> Ok, that explains it. I assumed s_min_extra_isize was meant to
> not be modifiable, and did not find a way to change it using the
> kernel or tune2fs, but now I can see that debugfs can set it.
>
>> If you want to pretend that file systems never get upgraded, then life
>> is much simpler. The general approach is that for less-sophisticated
>> customers (e.g., most people running enterprise distros) file system
>> upgrades are not a thing. But for sophisticated users, we do try to
>> make thing work for people who are aware of the risks / caveats /
>> rough edges. Google won't have been able to upgrade thousands and
>> thousands of servers in data centers all over the world if we limited
>> ourselves to Red Hat's support restrictions. Backup / reformat /
>> restore really isn't a practical rollout strategy for many exabytes of
>> file systems.
>>
>> It sounds like your safety checks / warnings are mostly targeted at
>> low-information customers, no?
>
> Yes, that seems like a reasonable compromise: just warn based
> on s_min_extra_isize, and assume that anyone who used debugfs
> to set s_min_extra_isize to a higher value from an ext3 file system
> during the migration to ext4 was aware of the risks already.
>
> That leaves the question of what we should set the s_time_gran
> and s_time_max to on a superblock with s_min_extra_isize<16
> and s_want_extra_isize>=16.
>
> If we base it on s_min_extra_isize, we never try to set a timestamp
> later than 2038 and so will never fail, but anyone with a grandfathered
> s_min_extra_isize from ext3 won't be able to set extended
> timestamps on any files any more. Based on s_want_extra_isize
> we would keep the current behavior, but could add a custom
> warning in the ext4 code about the small s_min_extra_isize
> indicating a theoretical problem.
I think it makes the most sense to always try to set timestamps on
inodes that have enough space for them. The chance of running into
a filesystem with 256-byte inode size but *no* space in the inode to
store an extended timestamp, but is *also* being modified by a new
kernel after 2038 is vanishingly small. This would require formatting
the filesystem with non-default mke2fs for ext3, using the filesystem
and storing enough xattrs on inodes that there isn't space for 12 bytes
of extra isize, and using it for 30+ years without upgrading to use
ext4 (which will also try to expand the inode to store the nsec
timestamps) and then modifying the inode after 2038.
Rather than printing a warning at mount time (which may be confusing
to users for a problem they may never see), it makes sense to only
print such a warning in the vanishingly small case that someone actually
tries to modify the inode timestamp but it doesn't fit, rather than on
the theoretical case that may never happen.
Cheers, Andreas
> Rather than printing a warning at mount time (which may be confusing
> to users for a problem they may never see), it makes sense to only
> print such a warning in the vanishingly small case that someone actually
> tries to modify the inode timestamp but it doesn't fit, rather than on
> the theoretical case that may never happen.
Arnd and I were discussing and we came to a similar conclusion that we
would not warn at mount. Arnd suggested maybe we include a
pr_warn_ratelimited() when we do write to these special inodes.
In general, there is an agreement to leave the fs granularity to a
higher resolution for such super blocks. And hence, the timestamps for
these special cases will never be clamped in memory.(Assuming we do
not want to make more changes and try to do something like
__ext4_expand_extra_isize() for in memory inode updates)
We could easily try to clamp these timestamps when they get written
out to the disk by modifying the EXT4_INODE_SET_XTIME to include such
a clamp. And, at this point we could also warn.
If this is acceptable, I could post an update.
-Deepa