test-fdutimensat failures after 7.0.6-200.fc44.x86_64 kernel update

test-fdutimensat failures after 7.0.6-200.fc44.x86_64 kernel update

[Collin Funk]

Hi,

After updating my Fedora system to kernel version 7.0.6-200.fc44.x86_64,
I noticed that the Gnulib 'test-fdutimensat' test fails a majority of
the time in my GNU coreutils checkout:

    $ seq 100 | xargs -n 1 sh -c '
      ./test-fdutimensat >/dev/null 2>&1
      echo $?
      ' | LC_ALL=C sort | LC_ALL=C uniq -c
         14 0
         86 134

Here is the section of code where it fails [1]:

  /* Play with UTIME_OMIT, UTIME_NOW.  */
  {
    struct stat st3;
    struct timespec ts[2];
    ts[0].tv_sec = BILLION;
    ts[0].tv_nsec = UTIME_OMIT;
    ts[1].tv_sec = 0;
    ts[1].tv_nsec = UTIME_NOW;
    nap ();
    ASSERT (func (BASE "link", ts) == 0);
    ASSERT (lstat (BASE "link", &st3) == 0);
    if (check_atime_on_symlinks)
      {
        /* The first assertion fails.  */
        ASSERT (st3.st_atime == Y2K);
        ASSERT (0 <= get_stat_atime_ns (&st3));
        ASSERT (get_stat_atime_ns (&st3) < BILLION / 2);
      }

Adding some print statements in place of the comment:

        fprintf (stderr, "%jd\n", (intmax_t) st3.st_atime);
        fprintf (stderr, "%jd\n", (intmax_t) Y2K);
        fprintf (stderr, "----------------\n");

Here is an example of what is seen when the test fails:

    $ ./test-fdutimensat 
    ----------------
    1778905091
    946684800
    test-lutimens.h:192: assertion 'st3.st_atime == Y2K' failed

Here is an example of what is seen when the test passes:

    $ ./test-fdutimensat 
    ----------------
    946684800
    946684800
    ----------------
    946684800
    946684800
    ----------------
    946684800
    946684800

So, it looks like the atime is being updated sometimes despite the use
of UTIME_OMIT.

Interestingly, in my Gnulib checkout residing in a separate directory
the test passes:

    $ git clone https://github.com/coreutils/gnulib.git
    $ cd gnulib
    $ ./gnulib-tool --create-testdir --dir testdir1 fdutimensat
    $ cd testdir1
    $ ./configure && make -j $(nproc)
    $ cd gltests
    $ seq 100 | xargs -n 1 sh -c '
          ./test-fdutimensat >/dev/null 2>&1
          echo $?
          ' | LC_ALL=C sort | LC_ALL=C uniq -c
        100 0

All of this testing was performed on the same BTRFS subvolume:

    /dev/mapper/nvme0n1p3_encrypted on /home type btrfs (rw,relatime,seclabel,ssd,space_cache=v2,subvolid=257,subvol=/root/home)

I could only find one commit since 6.19.14-300.fc44, which I never
experienced this error on, that looked like it may be related [2]. I am
very much unfamiliar with this code, so anything after this sentence
should be taken with a grain of salt.

Looking at the new inode_update_atime function that was added in that
commit:

> +static int inode_update_atime(struct inode *inode)
> +{
> +	struct timespec64 atime = inode_get_atime(inode);
> +	struct timespec64 now = current_time(inode);
> +
> +	if (timespec64_equal(&now, &atime))
> +		return 0;
> +
> +	inode_set_atime_to_ts(inode, now);
> +	return inode_time_dirty_flag(inode);
> +}

Compared to the old inode_update_timestamps function:

> -int inode_update_timestamps(struct inode *inode, int flags)
> +int inode_update_time(struct inode *inode, enum fs_update_time type,
> +		unsigned int flags)
>  {
> -	int updated = 0;
> -	struct timespec64 now;
> -
> -	if (flags & (S_MTIME|S_CTIME|S_VERSION)) {
> -		struct timespec64 ctime = inode_get_ctime(inode);
> -		struct timespec64 mtime = inode_get_mtime(inode);
> -
> -		now = inode_set_ctime_current(inode);
> -		if (!timespec64_equal(&now, &ctime))
> -			updated |= S_CTIME;
> -		if (!timespec64_equal(&now, &mtime)) {
> -			inode_set_mtime_to_ts(inode, now);
> -			updated |= S_MTIME;
> -		}
> -		if (IS_I_VERSION(inode) && inode_maybe_inc_iversion(inode, updated))
> -			updated |= S_VERSION;
> -	} else {
> -		now = current_time(inode);
> -	}
> -
> -	if (flags & S_ATIME) {
> -		struct timespec64 atime = inode_get_atime(inode);
> -
> -		if (!timespec64_equal(&now, &atime)) {
> -			inode_set_atime_to_ts(inode, now);
> -			updated |= S_ATIME;
> -		}
> +	switch (type) {
> +	case FS_UPD_ATIME:
> +		return inode_update_atime(inode);
> +	case FS_UPD_CMTIME:
> +		return inode_update_cmtime(inode);
> +	default:
> +		WARN_ON_ONCE(1);
> +		return -EIO;
>  	}
> -	return updated;
>  }

The behavior looks a bit different in the case of flags having S_MTIME,
S_CTIME, and S_VERSION being set. Before that commit it would use the
following to compare to atime:

    now = inode_set_ctime_current(inode);

However, now it will always use:

    struct timespec64 now = current_time(inode);

Could that be the cause of this behavior?

Let me know if there is any other useful information that I can share.

Thanks,
Collin

[1] https://github.com/coreutils/gnulib/blob/master/tests/test-lutimens.h#L176-L192
[2] https://github.com/torvalds/linux/commit/761475268fa8e322fe6b80bcf557dc65517df71e#diff-325e7194d6fde054508f3ab689c05b774e0755b85d5639dc92c19a2230bbcc2f