On Tue, Dec 03, 2024 at 04:12:45PM +1100, Dave Chinner wrote: > On Mon, Dec 02, 2024 at 12:55:13PM +0100, Christian Brauner wrote: > > On Sun, Dec 01, 2024 at 10:57:59PM -0500, Jon DeVree wrote: > > > When using the old mount API, the linux kernel displays a warning for > > > filesystems that lack support for timestamps after 2038. This same > > > warning does not display when using the new mount API > > > (fsopen/fsconfig/fsmount) > > > > > > util-linux 2.39 and higher use the new mount API when available which > > > means the warning is effectively invisible for distributions with the > > > newer util-linux. > > > > > > I noticed this after upgrading a box from Debian Bookworm to Trixie, but > > > its also reproducible with stock upstream kernels. > > > > > > From a box running a vanilla 6.1 kernel: > > > > > > With util-linux 2.38.1 (old mount API) > > > [11526.615241] loop0: detected capacity change from 0 to 6291456 > > > [11526.618049] XFS (loop0): Mounting V5 Filesystem > > > [11526.621376] XFS (loop0): Ending clean mount > > > [11526.621600] xfs filesystem being mounted at /mnt supports timestamps until 2038 (0x7fffffff) > > > [11530.275460] XFS (loop0): Unmounting Filesystem > > > > > > With util-linux 2.39.4 (new mount API) > > > [11544.063381] loop0: detected capacity change from 0 to 6291456 > > > [11544.066295] XFS (loop0): Mounting V5 Filesystem > > > [11544.069596] XFS (loop0): Ending clean mount > > > [11545.527687] XFS (loop0): Unmounting Filesystem > > > > > > With util-linux 2.40.2 (new mount API) > > > [11550.718647] loop0: detected capacity change from 0 to 6291456 > > > [11550.722105] XFS (loop0): Mounting V5 Filesystem > > > [11550.725297] XFS (loop0): Ending clean mount > > > [11552.009042] XFS (loop0): Unmounting Filesystem > > > > > > All of them were mounting the same filesystem image that was created > > > with: mkfs.xfs -m bigtime=0 > > > > With the new mount api the placement of the warning isn't clear: > > > > - If we warn at superblock creation time aka > > fsconfig(FSCONFIG_CMD_CREATE) time but it's misleading because neither > > a mount nor a mountpoint do exist. Hence, the format of the warning > > has to be different. > > > > - If we warn at fsmount() time a mount exists but the > > mountpoint isn't known yet as the mount is detached. This again means > > the format of the warning has to be different. > > > > - If we warn at move_mount() we know the mount and the mountpoint. So > > the format of the warning could be kept. > > > > But there are considerable downsides: > > > > * The fs_context isn't available to move_mount() > > which means we cannot log this into the fscontext as well as into > > dmesg which is annoying because it's likely that userspace wants to > > see that message in the fscontext log as well. > > > > * Once fsmount() has been called it is possible for > > userspace to interact with the filesystem (open and create > > files etc.). > > > > If userspace holds on to to the detached mount, i.e., never calls > > move_mount(), the warning would never be seen. > > > > * We'd have to differentiate between adding the first mount for a > > given filesystems and bind-mounts. > > > > IMHO, the best place to log a warning is either at fsmount() time or at > > superblock creation time > > It has to be done either during or after the ->fill_super() call > where the filesystems read their superblocks from disk and set up > the VFS superblock timestamp limits. > > Some of use filesystem developers wanted this timestamp warning to > be implemented in each filesystem ->fill_super method for this > reason - on-disk format information/warnings should be located with > the code that sets up the filesystem state from the on-disk > information. > > > but then the format of the warning will have to > > be slightly, changed. > > Yes please! > > This was the other main objection to a generic VFS timestamp warning > - inconsistent mount time log message formats. Filesytsems have > their own message formats with consistent identifiers, and that's > really what we should be using here. I don't mind moving this into individual fill_super() methods for the new mount api.