Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bigtime=1 for XFS by default #1650

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

I was randomly looking at kernel changes recently and noticed
that XFS today has the Year 2038 problem
by default.

While one would hope that most FCOS systems are deployed in a
reprovisionable environment, and are reprovisioned...let's still
be forward looking here and enable this by default - but only
for the testing-devel stream for now.

I was randomly looking at kernel changes recently and noticed
that XFS today has the [Year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem)
by default.

While one would hope that most FCOS systems are deployed in a
reprovisionable environment, and are reprovisioned...let's still
be forward looking here and enable this by default - but only
for the testing-devel stream for now.
@cgwalters cgwalters marked this pull request as draft March 31, 2022 22:05
@cgwalters
Copy link
Member Author

Requires coreos/coreos-assembler#2789

@bgilbert
Copy link
Contributor

Is the intention to promote this through the streams at the usual rate, or to bake in testing-devel for a while? If the former, we could potentially build the next stable with an older cosa and avoid adding the image.yaml setting.

@dustymabe
Copy link
Member

Is the intention to promote this through the streams at the usual rate, or to bake in testing-devel for a while? If the former, we could potentially build the next stable with an older cosa and avoid adding the image.yaml setting.

Along that same vein, do we see any potential risk here? If we do anything experimental that we don't plan to promote to stable in two weeks then I'd prefer to do it in next-devel/next.

IOW I'd like to keep testing exactly the same as the next stable.

@jlebon
Copy link
Member

jlebon commented Apr 1, 2022

It looks like our boot filesystem is also subject to the 2038 problem:

[root@cosa-devsh ~]# dumpe2fs /dev/disk/by-label/boot | grep 'Inode size'
Inode size:               128

mkfs.ext4(8) says:

File systems with an inode size of 128 bytes do not support timestamps beyond January 19, 2038. Inodes which are 256 bytes or larger will support extended timestamps, project id's, and the ability to store some extended attributes in the inode table for improved performance.

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.

I guess our bootfs' size is categorized as small. We could tweak it (-I 256), but the manpage also says:

The larger the inode-size the more space the inode table will consume, and this reduces the usable space in the filesystem and can also negatively impact performance.

So doesn't seem worth doing anything just yet.

Agreed with @dustymabe on letting it bake in next-devel first.

cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Apr 4, 2022
Creating this based on discussion in coreos/fedora-coreos-config#1650
around supporting bigtime for our XFS `/`.

The docs for this say:

> The larger the inode-size the more space the inode table will consume, and this reduces the usable space in the filesystem and can also negatively impact performance.

But for `/boot` we have a *tiny* number of files.  We also aren't
optimizing for performance here - `/boot` is rarely touched.

I think now is a good time to ensure we aren't subject to the year 2038
problem for it (as well as our other filesystems, but this one can
come first).
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Apr 4, 2022
Creating this based on discussion in coreos/fedora-coreos-config#1650
around supporting bigtime for our XFS `/`.

We want to enable gradual rollout of this; any changes today to
coreos-assembler's defaults become "insta stable" for FCOS and
current rhcos-4.11.  Having things stored in config git enables
CI and control.

Keep the default `0` to mean today's status quo (no bigtime).
We support opting-in just `/boot`, as well as doing both `/boot`
and `/`.

Re `/boot` which is ext4 today, the docs for this say:

> The larger the inode-size the more space the inode table will consume, and this reduces the usable space in the filesystem and can also negatively impact performance.

But for `/boot` we have a *tiny* number of files.  We also aren't
optimizing for performance here - `/boot` is rarely touched.

I think now is a good time to ensure we aren't subject to the year 2038
problem for it (as well as our other filesystems, but this one can
come first).

Then we can do `/` too.
@cgwalters
Copy link
Member Author

@dustymabe
Copy link
Member

I'm wondering if this isn't a change we should push Fedora to make at a project level rather than one-offing it here.

IOW what are the reasons (if any) Fedora isn't doing this already?

@cgwalters
Copy link
Member Author

I'm wondering if this isn't a change we should push Fedora to make at a project level rather than one-offing it here.

Agree, it likely makes sense to expand this beyond FCOS. However - are we leaders or followers? I think it depends. But on things like this, it can make some sense for us to lead.

IOW what are the reasons (if any) Fedora isn't doing this already?

We're Fedora too, so I'd say "other editions/builds" or so. I suspect the answer is just that "no one thought of it until now". But I can ask around!

@cgwalters
Copy link
Member Author

On the other hand, Workstation defaulting to btrfs (which does not have this problem by default at least today, and that may have been true for years) means it is a bit more on us.

@dustymabe
Copy link
Member

Agree, it likely makes sense to expand this beyond FCOS. However - are we leaders or followers? I think it depends. But on things like this, it can make some sense for us to lead.

I think we can lead, but I view this a lot like Red Hat's "upstream first" mentality. In general we don't want to just make changes ourselves and not get them back "upstream" such that we diverge from other editions. If we do that we're adding to our maintenance burden and it's unsustainable (FCOS will suffer in one way or another).

So typically what happens if there is a change we'd want I'd say let's propose it as a change in all of Fedora and hopefully it gets accepted. Once accepted we don't have to wait for the next major version of Fedora, we can "backport" that change to FCOS now if we want.

However, if the change doesn't get accepted we need to take a long hard look at why. Are there good reasons that apply to us too? Ultimately we can decide to make the change anyway, but we're much wiser for going through the process.

@sandeen
Copy link

sandeen commented Apr 6, 2022

As always, please feel free to reach out to the filesystem experts who are close at hand for questions like this.

bigtime will be enabled on xfs by default as of the xfsprogs-5.15.0 release which should happen very soon. (I'm late thanks to PTO.) I'll be happy to push that releases to existing Fedora releases as needed.

As for ext4, I'll need to dig a little more, but I believe it's automatically choosing the "small" profile for small filesystems, which leads to the small inodes and smaller timestamps. If typical boot filesystems encounter this problem it might be worth bringing this issue up on the ext devel list.

Thanks,
-Eric

@sandeen
Copy link

sandeen commented Apr 7, 2022

xfsprogs-5.15.0 with bigtime on by default is now built in rawhide. (It also provides an upgrade path for older filesystems.)

And - ext4 chooses the "small" profile and smaller inodes for anything under 512MB. 512MB and above should get larger inodes, along with extended timestamps.

-Eric

@dustymabe
Copy link
Member

Thanks @sandeen!

@cgwalters - For XFS should we just let this passively come to us?

For ext4, should we consider forcing it to use a different profile even though we're under 512M for the boot FS? Alternatively we could consider bumping our /boot FS to 512M.

@bgilbert
Copy link
Contributor

bgilbert commented Apr 7, 2022

Resizing a default filesystem would have implications for the Butane RAID template, and for our minimum recommended disk size such that the root filesystem is kept ≥ 8 GiB.

@dustymabe
Copy link
Member

I'm going to close this as it seems like the change is coming to us anyway. If we want to do anything for ext4 let's open a tracker issue and discuss what to do there.

@dustymabe dustymabe closed this May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants